Describe the bug
We lint more files than we need to, when VALIDATE_ALL_CODEBASE is set to false.
It appears that, rather than looking at changed files in that branch/pull request, it looks at all differences between the DEFAULT_BRANCH and the current branch and uses them to select the files to lint. This means if you have not merged with the DEFAULT_BRANCH recently, then you will lint old copies of files which have subsequently been changed in the DEFAULT_BRANCH.
To Reproduce
Steps to reproduce the behavior:
feature1 and edit some code.DEFAULT_BRANCH (e.g. edit theREADME.md file).feature1feature1README.md was linted despite not being changed in feature1 branch.Expected behavior
I would expect only file changes in the feature1 branch to be linted.
It's a bit confusing that a file I have not touched, and that was in fact changed in a separate branch to mine, to be linted.
In fact, if the commit outside of this branch was to fix a linting problem, then your branch may suddenly show linting errors just by merging that fixed file into the DEFAULT_BRANCH. The fix is easy - merge the latest DEFAULT_BRANCH into feature1 branch, but may not be the most obvious.
Also a possible fix (if that's not an intended behaviour) is to change the diff command to:
git diff --name-only '${DEFAULT_BRANCH}...${GITHUB_SHA}' --diff-filter=d
Note the additional dot between the default branch name and commit SHA, see git diff docs.
@bazzadp @dec5e Great conversation on this...
The Pull payload doesn't give us enough information to see what the root of the PR would be, so we had to assume the default branch. This with the .. diff seems to be a bug and we really want ... as this will give a more fair assessment of the files changed.
@admiralAwkbar could you please leave a comment here when this fix would be released? Thanks in advance
And huge thanks for such a quick fix.
@dec5e The fix is live if you're using docker://github/super-linter:latest but likely later this week we will create a new release and have it formally tagged :)
@admiralAwkbar unfortunately problem is still there. I've created a test repo to show it, check this PR https://github.com/dec5e/super-linter-test/pull/2 and linter results. I've doublechecked that correct version of the linter is used: image hash is 11ec9aa31cdc which matches v3.8.3, the fix was released under v3.7.0.
Even though locally I'm getting only correct files with the git diff command, linter has more files. And I can't test linter locally to see if it is linter issue or github actions doing something wrong, because with RUN_LOCAL=true all files are checked and necessary code block is skipped.
Do you have any ideas, what's wrong here? or maybe I have invalid workflow configuration?
@admiralAwkbar did you have a chance to check my example reproducing the bug?
Most helpful comment
@bazzadp @dec5e Great conversation on this...
Overview
The Pull payload doesn't give us enough information to see what the root of the PR would be, so we had to assume the default branch. This with the
..diff seems to be a bug and we really want...as this will give a more fair assessment of the files changed.