Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update code and tests post a linting check. #508

Merged
merged 11 commits into from
Apr 3, 2019
Merged

Update code and tests post a linting check. #508

merged 11 commits into from
Apr 3, 2019

Conversation

ozym
Copy link
Contributor

@ozym ozym commented Apr 1, 2019

golangci-lint run -E gosec

Spotted one error related to reuse of a variable, but mostly error handling in tests and deadcode.

@danieldooley @junghao could you have a look through at some point. The only outstanding warning is about the last few lines of tools/sit/main.go (error handling in the "verbose" part).

@ozym ozym added the bug Something isn't right label Apr 1, 2019
@ozym ozym self-assigned this Apr 1, 2019
@junghao
Copy link
Contributor

junghao commented Apr 1, 2019

Looks good. But TravisCI has problem in testing?
And do you want to add golangci-lint to .travis.yml ?

@ozym
Copy link
Contributor Author

ozym commented Apr 2, 2019

Thanks @junghao just waiting on getting the code valid first before adding to travis. That will be the next step after all the code passes muster. Kicked travis to re-run the checks and it seems to have come back.

junghao
junghao previously approved these changes Apr 2, 2019
Copy link
Contributor

@junghao junghao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@danieldooley
Copy link
Contributor

@ozym hope you don't mind, I've fixed the tools/sit linting and enabled the linter in the travis build

@ozym
Copy link
Contributor Author

ozym commented Apr 3, 2019

@danieldooley no problems, will need to be reviewed / ticked again.

@ozym ozym merged commit a86ef56 into GeoNet:master Apr 3, 2019
@ozym ozym deleted the lint branch April 3, 2019 23:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't right
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants