ci: clang_format.yml lints only modified source files #196
Conversation
|
Leaving draft PR up for now, will finalize the PR tomorrow. |
rjwignar
changed the title
rjwignar
changed the title
| id: changed-files | ||
| uses: tj-actions/changed-files@v42.0.5 | ||
| with: | ||
| files: | |
There was a problem hiding this comment.
This glob needs to be improved, we also have cpp source files directly under src (Not only features subfolder). Also under include
Can look here for some glob hints
https://github.com/doodlum/skyrim-community-shaders/blob/dev/cmake/AddCXXFiles.cmake
There was a problem hiding this comment.
@FlayaN you think you can handle the globs easily?
There was a problem hiding this comment.
Hello! I don't have time for the next 2 weeks to fix the globs (And test them). I can however guess :P
Looking at documentation at: https://github.com/tj-actions/changed-files
It looks like they support the src/** glob syntax.
So my guess for a good glob would be:
files: |
src/**
include/**
**/*.hlsl
**/*.hlsliThere was a problem hiding this comment.
@FlayaN since you're on a roll, you want to take this over the line?
There was a problem hiding this comment.
I can give this another look in a couple days
There was a problem hiding this comment.
I recently updated the glob with the help of https://codepen.io/mrmlnc/pen/OXQjMe:
files: |
src/**/*.{cpp,h}
include/**/*.{h,hpp}
**/*.hlsl
**/*.hlsli
Here's a couple example runs of the updated glob on rjwignar/issue-128-test:
| Test | Commit | Clang-Format Job |
|---|---|---|
Update cpp/h files in src |
rjwignar@c17a2f6 | https://github.com/rjwignar/skyrim-community-shaders/actions/runs/9409659767/job/25920004938 |
Update h/hpp files in include |
rjwignar@51b9322 | https://github.com/rjwignar/skyrim-community-shaders/actions/runs/9409737196/job/25920220892 |
The recent force-push was due to a rebase of my branch (rjwignar/issue-128) on top of the latest dev branch
Please let me know if additional changes are required.
|
Thanks again. Please note I am waiting for the issues raised by @FlayaN to be resolved before merging. |
|
@rjwignar thanks again. Just checking to see if you think you'll be able to address or if we should merge and someone else will pick up the other source code locations. |
|
Hi @alandtse thanks for reaching out. I've been meaning to update the glob but I've been bogged down in class commitments. If the team can wait until then we can keep this open. |
to list modified source files
…c folder and h/hpp files inside include folder
rjwignar
force-pushed
the
rjwignar/issue-128
branch
from
9088c8a to
9e1a4ff
Compare
|
Thanks. We're in a major refactor right now but after that's in, I'll see about merging. |
|
Ok, refactor is done. Thanks again for the help! |
This fixes #128
Summary
Added two steps to
.github/workflows/clang_format.ymlto lint only modified C++ source files and shader files (.hlsli, .hlsl)changed-filesThe
changed-filesstep uses tj-actions/changed-files to retrieve a comma-separated list of changed C++ and shader files. The comma-separated list is then processed and passed to theFormat changed filesstepprocess-listThe
process-liststep processes the filelist to be properly passed to DoozyX/clang-format-lint-actionfilepaths with whitespaces are made unix-friendly
(e.g.,
Complex Parallax Materials/file.hlsl-->Complex\ Parallax\ Materials/file.hlsl)commas replaced with whitespaces to separate multiple filepaths
(e.g.,
file.hlsl,src/Features/DistantTreeLighting.cpp-->file.hlsl src/Features/DistantTreeLighting.cpp)Changes to
Format changed filesstepUpgraded DoozyX/clang-format-lint-action from v0.16.2 to v0.17 to allow filepaths containing whitespaces
This step now only executes if the
changed-filesstep retrieves any modified files.Test runs
A test version of this workflow is available at rjwignar/issue-128-test, where rjwignar/issue-128-test was added as a triggering branch.
Here are some example runs: