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 listing of modified files #3472

Merged
merged 5 commits into from
Apr 16, 2024
Merged

Fix listing of modified files #3472

merged 5 commits into from
Apr 16, 2024

Conversation

vkucera
Copy link
Contributor

@vkucera vkucera commented Apr 5, 2024

Consider the merge-base commit as the starting point of diff instead of listing all differences between branches.

Fixes #2125

Proposed Changes

MegaLinter currently gets the list of files to check by comparing HEAD and the base branch.

  • That is fine for pull_request events where HEAD is the merge commit and git diff only lists files in the PR.
  • It is not fine for push events where the base branch can contain commits not present in the head branch and git diff also lists files from those commits.

To list only files from commits in the head branch which are not present in the base branch, one should run git diff against the common ancestor (aka merge-base) of the two branches.
This can be done with git diff <base-branch>... which is equivalent to:

  • git diff <base-branch>...HEAD
  • git diff --merge-base <base-branch> HEAD
  • git diff $(git merge-base <base-branch> HEAD) HEAD

Readiness Checklist

Author/Contributor

  • Add entry to the CHANGELOG listing the change and linking to the corresponding issue (if appropriate)
  • If documentation is needed for this change, has that been included in this pull request

Reviewing Maintainer

  • Label as breaking if this is a large fundamental change
  • Label as either automation, bug, documentation, enhancement, infrastructure, or performance

Consider the merge-base commit as the starting point of diff instead of listing all differences between branches.
@vkucera
Copy link
Contributor Author

vkucera commented Apr 8, 2024

The test failed after 5e1adac because of a missing merge base which I fixed by adding fetch-depth: 0 in the checkout step of that action.
Now the test is failing at test_get_linter_help but I don't understand the issue.

@nvuillam
Copy link
Member

nvuillam commented Apr 8, 2024

it seems everything is working :)

We'll merge this one once the current CI issue with eslint-plugin-json is fixed :)

@vkucera
Copy link
Contributor Author

vkucera commented Apr 10, 2024

it seems everything is working :)

We'll merge this one once the current CI issue with eslint-plugin-json is fixed :)

Thanks @nvuillam , which issue are you referring to?

@nvuillam
Copy link
Member

@vkucera it's fixed, i merge once CI is ok (and it should be ^^ )

@echoix echoix merged commit 85c5718 into oxsecurity:main Apr 16, 2024
6 checks passed
@vkucera vkucera deleted the fix-diff branch April 16, 2024 11:37
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.

Listing modified files does not work when running on the pull_request_target event
3 participants