[Github] Make PR formatting job only run with C/C++ changes #69556
Conversation
Currently the PR formatting job only runs clang-format. There isn't a lot of utility in running it if there aren't any C/C++ changes as there will be nothing to format. This isn't super noisy currently as the job doesn't fail if there aren't any C/C++ changes, but it's a bit of a waste. In addition, this patch names the code formatting job "Check C++ Formatting" to make it clear that this job only checks C/C++ formatting rather than Python formatting/other languages.
| I'm guessing the plan is to extend this to check formatting with black at some point too? |
| @llvm/pr-subscribers-github-workflow Author: Aiden Grossman (boomanaiden154) ChangesCurrently the PR formatting job only runs clang-format. There isn't a lot of utility in running it if there aren't any C/C++ changes as there will be nothing to format. This isn't super noisy currently as the job doesn't fail if there aren't any C/C++ changes, but it's a bit of a waste. In addition, this patch names the code formatting job "Check C++ Formatting" to make it clear that this job only checks C/C++ formatting rather than Python formatting/other languages. Full diff: https://github.com/llvm/llvm-project/pull/69556.diff 1 Files Affected:
diff --git a/.github/workflows/pr-code-format.yml b/.github/workflows/pr-code-format.yml index 3a91ffb0b1ad9a7..060646df6ae4759 100644 --- a/.github/workflows/pr-code-format.yml +++ b/.github/workflows/pr-code-format.yml @@ -1,10 +1,19 @@ name: "Check code formatting" -on: pull_request_target + +on: + pull_request_target: + paths: + - '**/*.cpp' + - '**/*.c' + - '**/*.h' + - '**/*.inc' + permissions: pull-requests: write jobs: - code_formatter: + cpp_code_formatter: + name: "Check C++ Formatting" runs-on: ubuntu-latest steps: - name: Fetch LLVM sources |
tstellar left a comment
tstellar left a comment There was a problem hiding this comment.
This job needs to be re-written to not use pull_request_target, but that's unrelated to this change. LGTM.
| Please revert or extend since this now doesn't run on python files. The code formatter runs both formatting which is why I opted to not have a filter here since that means we need to maintain several lists of extensions. |
| Sorry. Reverted in cd205ef. I'll post another version of this patch with the relevant python extensions when I have a chance. |
Local branch amd-gfx 634c100 Merged main:817519058a98 into amd-gfx:7e00bdde62c2 Remote branch main 80b2aac [Github] Make PR formatting job only run with C/C++ changes (llvm#69556)
4 participants
Currently the PR formatting job only runs clang-format. There isn't a lot of utility in running it if there aren't any C/C++ changes as there will be nothing to format. This isn't super noisy currently as the job doesn't fail if there aren't any C/C++ changes, but it's a bit of a waste.
In addition, this patch names the code formatting job "Check C++ Formatting" to make it clear that this job only checks C/C++ formatting rather than Python formatting/other languages.