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

[Github] Set start rev to merge base in code format action #75132

Merged

Conversation

boomanaiden154
Copy link
Contributor

This patch sets the start revision to the merge base so that the c++ formatting action won't produce any diffs related to changes in main but not in the PR branch. This also leaves a TODO to migrate over to the --diff_from_common_commit option in git-clang-format once LLVM v18 is released.

This patch sets the start revision to the merge base so that the c++
formatting action won't produce any diffs related to changes in main but
not in the PR branch. This also leaves a TODO to migrate over to the
--diff_from_common_commit option in git-clang-format once LLVM v18 is
released.
@llvmbot
Copy link
Collaborator

llvmbot commented Dec 12, 2023

@llvm/pr-subscribers-github-workflow

Author: Aiden Grossman (boomanaiden154)

Changes

This patch sets the start revision to the merge base so that the c++ formatting action won't produce any diffs related to changes in main but not in the PR branch. This also leaves a TODO to migrate over to the --diff_from_common_commit option in git-clang-format once LLVM v18 is released.


Full diff: https://github.com/llvm/llvm-project/pull/75132.diff

1 Files Affected:

  • (modified) .github/workflows/pr-code-format.yml (+5-1)
diff --git a/.github/workflows/pr-code-format.yml b/.github/workflows/pr-code-format.yml
index c27c282eb2a19a..5223089ee8a93d 100644
--- a/.github/workflows/pr-code-format.yml
+++ b/.github/workflows/pr-code-format.yml
@@ -67,10 +67,14 @@ jobs:
           START_REV: ${{ github.event.pull_request.base.sha }}
           END_REV: ${{ github.event.pull_request.head.sha }}
           CHANGED_FILES: ${{ steps.changed-files.outputs.all_changed_files }}
+        # TODO(boomanaiden154): Once clang v18 is released, we should be able
+        # to take advantage of the new --diff_from_common_commit option
+        # explicitly in code-format-helper.py and not have to diff starting at
+        # the merge base.
         run: |
           python ./code-format-tools/llvm/utils/git/code-format-helper.py \
             --token ${{ secrets.GITHUB_TOKEN }} \
             --issue-number $GITHUB_PR_NUMBER \
-            --start-rev $START_REV \
+            --start-rev $(git merge-base $START_REV $END_REV) \
             --end-rev $END_REV \
             --changed-files "$CHANGED_FILES"

Copy link
Member

@jyknight jyknight left a comment

Choose a reason for hiding this comment

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

Thanks!

Copy link
Collaborator

@tru tru left a comment

Choose a reason for hiding this comment

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

LGTM. I hate how difficult it is to correctly figure out the diff here. Thanks for persevering

@boomanaiden154 boomanaiden154 merged commit b3af755 into llvm:main Dec 12, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants