-
Notifications
You must be signed in to change notification settings - Fork 475
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
Fail CI build if golint finds problems (non-functional) #524
Comments
Agree, approved |
Somewhat related note, ever use codeclimate or tools like that? I prefer local tools myself, but wondering if in addition having codeclimate provides clearer visibility to contributors. |
I tried Code Climate recently, actually. I have used SonarQube in the past as well and adored the visibility and accountability it brought into our workflow. I can play around with https://docs.codeclimate.com/docs/github-pull-requests on my fork and show you examples and my configuration if you like it. In theory, Code Climate and golint overlap a lot so only one of them would be absolutely needed. I would still want to leave golint in Plus Code Climate does Unit Test coverage report 🎉 This will take a while to get all the problems under control so I might break it into more than a single PR, was I able to through it, to keep merge conflicts at a minimum. |
I played around with Code Climate and I have fast proof of concept available for feedback: https://codeclimate.com/github/pedroMMM/goss/issues As suspected Code Climate also supports sourcing problems from gofmt, golint, govet. This would enable local testing but also centralizing information on Code Climate and PR validation. The big issue here is that with all of those sources enabled Code Climate is reporting 432 issues. Fixing all of this would be an insane PR! I suggest evaluate my Code Climate concept before anything else. Then if you still want to continue down this path (which I think we should) I would create a PR and work with you to configure Code Climate on this repo. Only then should anyone start working on the cleanup, which could be scoped to packages or files to minimize confusion and even encourage the community to contribute. |
@aelsabbahy I am tagging you in case you have missed my last post. Let me know if you want to reduce scope or any forward steps. |
I agree 100% with your last post. Definitely, should have the ability to locally test. To be honest with you, I haven't had the time to do a thorough review on this with the holidays and all, that's why I haven't responded. I'll go through this in detail probably sometime in the first two weeks of January. At a high level:
Happy holidays, and thanks for all your great contributions so far! |
That is more than fair, I am sorry if made you feel pressured. There is a lot to unpack in the Code Commit report, I was just making sure it didn't get lost. I just want to make clear that not all the problems there are possible to analyze locally. Code Commit adds a little bit of their analysis on top of gofmt, golint, govet. As soon as I can I will get a PR with the configuration changes and instructions on how to get Code Commit configured on your side. |
* Add Code Coverage Support * Add codeclimate token Co-authored-by: Ahmed Elsabbahy <[email protected]>
* Add Code Coverage Support * Add codeclimate token Co-authored-by: Ahmed Elsabbahy <[email protected]>
Describe the feature:
Golint should find 0 problems or fail the CI build.
Describe the solution you'd like
While it's non-functional, it's worrying that golint finds a high quantity of problems during builds.
Since most problems seem to be documentation, it will double as a method to lower the entry barrier to understand Goss internals.
Describe alternatives you've considered
Some actions can be taken to minimize the issue:
Go vet should also be enabled as part of the build.
The text was updated successfully, but these errors were encountered: