Run go vet as part of the Travis CI.
#626
Conversation
Also fix existing issues found by Go vet.
|
Thanks for taking care of this. It seems we were swallowing some errors on the CLI? |
.travis.yml
Outdated
| @@ -48,4 +48,5 @@ script: | |||
| - cd $TRAVIS_BUILD_DIR/backend/src | |||
| - gimme -f 1.11.4 | |||
| - source ~/.gimme/envs/go1.11.4.env | |||
| - go vet -shadow ./... | |||
There was a problem hiding this comment.
I think this means only run the shadow checks?
You might need to vet shadows separately on a second invokation, since the flag appears to be still experimental.
There was a problem hiding this comment.
Thanks, good catch! I verified that go vet -all -shadow runs all checks including shadow.
|
Thanks for the pointers @yebrahim. PTAL. |
|
Sweet, thanks! |
| reader, err := os.Open(config.File) | ||
| if err != nil { | ||
| return errors.New(fmt.Sprintf("Failed to load sample %s. Error: %v", config.Name, err.Error())) | ||
| reader, configErr := os.Open(config.File) |
There was a problem hiding this comment.
will this code not pass go vet check without changing it? configErr doesn't look like a right name.
There was a problem hiding this comment.
Yeah, here, I think the vet error is spurious but the fix doesn't harm anything IMO. It complains as err shadows the previous declaration outside the for loop. configErr is just a variable name related to errors opening and reading the config file. I can rename it if you like. What would you suggest?
| @@ -52,15 +52,16 @@ func (s *DBStatusStore) InitializeDBStatusTable() error { | |||
|
|
|||
| // The table is not initialized | |||
| if !rows.Next() { | |||
| sql, args, err := sq. | |||
| sql, args, queryErr := sq. | |||
There was a problem hiding this comment.
why only this place but not others similiar places?
There was a problem hiding this comment.
This is the only one go vet complains about since it's a redeclaration of err due to the other variables on the same line.
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: vicaire The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: vicaire The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Looks like this PR broke the Travis tests. #646 |
Also fix existing issues found by Go vet.
This change is