-
Notifications
You must be signed in to change notification settings - Fork 4.4k
ci(security-guardian): use triple dot to get PR diff changes #36588
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
Conversation
It was incorrectly assumed that `git diff A..B` would behave similarly to `git log A..B`, but that is not the case. Counterintuitively, it behaves like `git log A...B`, getting changes from the main branch as well, which is not what we want. The correct command is `git diff A...B`. This behaves like `git log A..B` as expected. See https://stackoverflow.com/questions/7251477/what-are-the-differences-between-double-dot-and-triple-dot-in-git-dif/46345364#46345364
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.
(This review is outdated)
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
|
@Mergifyio update |
✅ Branch has been successfully updated |
Merge Queue Status🚫 The pull request has left the queue (rule: This pull request spent 42 seconds in the queue, with no time running CI. ReasonThe pull request #36588 has been manually updated HintIf you want to requeue this pull request, you can post a |
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
Merge Queue Status✅ The pull request has been merged at 70f8f3c This pull request spent 32 minutes 33 seconds in the queue, including 31 minutes 39 seconds running CI. Required conditions to merge
|
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
|
Comments on closed issues and PRs are hard for our team to see. |
Reason for this change
It was incorrectly assumed that
git diff A..Bwould behave similarly togit log A..B, but that is not the case. Counterintuitively, it behaves likegit log A...B, getting changes from the main branch as well, which is not what we want.See https://stackoverflow.com/questions/7251477/what-are-the-differences-between-double-dot-and-triple-dot-in-git-dif/46345364#46345364
Description of changes
The correct command is
git diff A...B. This behaves likegit log A..Bas expected.Describe any new or updated permissions being added
No new permissions are added.
Description of how you validated changes
Ran security guardian build and tests to verify
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license