Misc fixes and refactoring #1148
Conversation
Reduced struct of size from 264 bytes to 256 bytes
julienschmidt
force-pushed
the
misc_fixes
branch
from
f8c290d to
35f1118
Compare
|
LGTM. But 0ea749d seems too strict to me. I usually use this variable hiding. Which linter do you want to use? |
|
According to the documentation comment of
I think alignment with If these changes are merged, it will be difficult to check what is changed from reference implementation. |
| if err = parseDSNParams(cfg, dsn[j+1:]); err != nil { | ||
| return |
There was a problem hiding this comment.
Why not return values like this here for consistency?
return cfg, err
| var name string | ||
| name, err = url.QueryUnescape(value) | ||
| if err != nil { | ||
| return fmt.Errorf("invalid value for server pub key name: %v", err) |
There was a problem hiding this comment.
Linter message:
non-wrapping format verb for fmt.Errorf. Use %w to format errors (errorlint)
return fmt.Errorf("invalid value for server pub key name: %w", err)
There was a problem hiding this comment.
In this case this is just a general DSN syntax error. The decoding error shouldn't be wrapped.
| } | ||
|
|
||
| return | ||
| return err |
There was a problem hiding this comment.
It may be better to use return nil, because no error is returned in this case.
|
@julienschmidt It would be better to resubmit each lint error fix as separate MR. It would be easier to review and accept at least some of them. |
Description
This PR includes a collection of different kinds of fixes (see commit messages below) to issues found with the help of several linters, ranging from missing error checks, to resource leaks because of unclosed rows / statements (luckily only in tests), a benchmark calling a wrong function, typos, shadowing issues and scope issues to bad coding style / formatting issues.
This PR also serves as a preparation for adding better linters to our CI pipeline.
Checklist