Remove lint exceptions and fix remaining issues

google / certificate-transparency-go

Auditing for TLS certificates (Go code)

https://certificate.transparency.dev

Apache License 2.0

867 stars 230 forks source link

Remove lint exceptions and fix remaining issues #1438

Closed silaselisha closed 2 months ago

silaselisha commented 3 months ago

[x] I have removed lint exception by commenting it out
[x] Fixed all remaining issues regarding golangci-lint exception

silaselisha commented 3 months ago

@roger2hk in the previous PR created to solve issues #1438 I realized it was failing as a result of a file of type *os.File getting closed twice and that's in file loglist_refresher_test.go on function createTempFile.

AlCutter commented 3 months ago

Thanks @silaselisha, I think we might want to "downgrade" some of these exits/panics - I think it's fine to do that in tests, but particularly in longer-lived server processes or client libraries it's likely better to just log these.

silaselisha commented 3 months ago

@AlCutter will it be fine for the recommended "downgrading" changes being part of this PR?

silaselisha commented 3 months ago

@roger2hk I'll look into it A.S.P. and also should I make changes to pieces of code that I didn't introduce but broke this rule https://google.github.io/styleguide/go/decisions#error-strings?

roger2hk commented 3 months ago

@roger2hk I'll look into it A.S.P. and also should I make changes to pieces of code that I didn't introduce but broke this rule https://google.github.io/styleguide/go/decisions#error-strings?

You should scope it to the lint changes. If you find some other error strings and would like to make changes, please open a separate pull request to address them.

AlCutter commented 3 months ago

@AlCutter will it be fine for the recommended "downgrading" changes being part of this PR?

@silaselisha yes please