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

Implement Static Analysis and Fix Bugs #38

Merged
merged 18 commits into from
Jun 23, 2017
Merged

Implement Static Analysis and Fix Bugs #38

merged 18 commits into from
Jun 23, 2017

Conversation

opalmer
Copy link
Contributor

@opalmer opalmer commented Jun 18, 2017

This PR adds gometalinter to the project and fixes problems it has found in the process. This PR was created in response to comments made on #37 by @shurcooL to not only make the requested fixes but ensure we don't have these kinds of issues again in the future.

The changes introduced in this PR are documented in the CHANGELOG.

@codecov-io
Copy link

codecov-io commented Jun 18, 2017

Codecov Report

Merging #38 into master will decrease coverage by 0.27%.
The diff coverage is 45.45%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #38      +/-   ##
==========================================
- Coverage   18.06%   17.79%   -0.28%     
==========================================
  Files          21       21              
  Lines        1738     1742       +4     
==========================================
- Hits          314      310       -4     
- Misses       1394     1398       +4     
- Partials       30       34       +4
Impacted Files Coverage Δ
changes.go 4.63% <ø> (ø) ⬆️
types.go 92.85% <ø> (ø) ⬆️
changes_revision.go 0% <0%> (ø) ⬆️
groups_include.go 0% <0%> (ø) ⬆️
changes_edit.go 0% <0%> (ø) ⬆️
events.go 69.23% <100%> (ø) ⬆️
authentication.go 76% <33.33%> (-7.34%) ⬇️
gerrit.go 81.95% <55.55%> (-0.9%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 195c9cf...709c69f. Read the comment docs.

@opalmer
Copy link
Contributor Author

opalmer commented Jun 18, 2017

@andygrunwald / @shurcooL please take a look. The only change here that might be worth reverting/working around is testing against Go 1.5. That said if the tests pass on other versions of Go they should pass on 1.5 too (we just can't lint against 1.5).

@opalmer opalmer added this to the 0.4.0 milestone Jun 18, 2017
@opalmer opalmer self-assigned this Jun 19, 2017
@andygrunwald andygrunwald merged commit fc60824 into master Jun 23, 2017
@andygrunwald andygrunwald deleted the add-linters branch June 23, 2017 16:56
@andygrunwald
Copy link
Owner

Thanks a lot for this.
Dropping Go 1.5 is fine for me. Go 1.6 is out for quite some time.

This covers a few bugs and fixes them. Really nice. Thanks for your work @opalmer and for triggering this @shurcooL

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants