- Notifications
You must be signed in to change notification settings - Fork967
Fix for #4678: top-level 'make lint' wasn't working#4679
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:dev
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
The revive command seems to have had a syntax error in the fileinput glob. It appears to have been broken in a way that did notresult in a return code being set. This change uses 'find' tobuild the input to the linter.Note that it is expected to fail the CI script, because it isuncovering some existing lint issues that were not being caught.
GNUmakefile Outdated
| @@ -1000,14 +1000,18 @@ endif | |||
| tools: | |||
| cd internal/tools && go generate -tags tools ./ | |||
| LINTFILESCMD=find src/os/ src/reflect/ -type f -name '*.go' | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
How about changing this variable to contain a list of directories to scan? That would be easier to maintain/adjust/expand in future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@b0ch3nski Similar to this?7190e01
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@cibomahto LGTM 👍
This PR seems like a good thing, but then suddenly will require a followup to fix all of the issues it raises, if I am not mistaken? |
You are correct! If the issues that this surfaces aren't important, then an alternative is to just delete this test. |
Uh oh!
There was an error while loading.Please reload this page.
As noted in#4678, the revive command seems to have had a syntax error in the file input glob. It appears to have been broken in a way that did not result in a return code being set. This change uses 'find' to build the input to the linter.
Note that it is expected to fail the CI script, because it is uncovering some existing lint issues that were not being caught.