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

Fix for #4678: top-level 'make lint' wasn't working #4679

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

cibomahto
Copy link
Contributor

@cibomahto cibomahto commented Jan 2, 2025

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.

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.
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'

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.

Copy link
Contributor Author

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

Choose a reason for hiding this comment

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

@cibomahto LGTM 👍

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

Successfully merging this pull request may close these issues.

2 participants