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.

Sign up for GitHub

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

ci: clang_format.yml lints only modified source files #196

Merged
merged 8 commits into from
Jul 4, 2024
Merged

ci: clang_format.yml lints only modified source files #196

Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
f6f28b8
updated DoozyX/clang-format-lint to v0.17
rjwignar Feb 27, 2024
848f7bf
added tj-actions/changed-files step
rjwignar Feb 27, 2024
3517348
updated changed-files job with shader files and separator
rjwignar Feb 29, 2024
6716236
updated changed-files step name, added process-list step
rjwignar Feb 29, 2024
61dcc37
updated clang-format-lint action step to run only if there are modifi…
rjwignar Feb 29, 2024
b3398f0
modified format step to format only modified source files
rjwignar Feb 29, 2024
bd37776
updated commments
rjwignar Feb 29, 2024
9e1a4ff
updated changed-files globs to include all cpp (cpp/h) files under sr…
rjwignar Jun 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 23 additions & 2 deletions .github/workflows/clang_format.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,31 @@ jobs:
# make sure the parent commit is grabbed as well, because
# that's what will get formatted (i.e. the most recent commit)
fetch-depth: 2
# retrieve comma-separated list of modified files
- name: Retrieve changed files
id: changed-files
uses: tj-actions/changed-files@v42.0.5
with:
files: |
Copy link
Collaborator

@FlayaN FlayaN Feb 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

@FlayaN you think you can handle the globs easily?

Copy link
Collaborator

@FlayaN FlayaN Mar 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

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
  **/*.hlsli

Copy link
Collaborator

Choose a reason for hiding this comment

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

@FlayaN since you're on a roll, you want to take this over the line?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can give this another look in a couple days

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

src/**/*.{cpp,h}
include/**/*.{h,hpp}
**/*.hlsl
**/*.hlsli
separator: ","
# process modified files list before passing to clang-format-lint-action
- name: Process changed files list
id: process-list
run: |
CHANGED_FILES="${{ steps.changed-files.outputs.all_changed_files }}"
CHANGED_FILES="${CHANGED_FILES// /\\ }"
CHANGED_FILES=$(echo "$CHANGED_FILES" | tr ',' ' ')
echo "::set-output name=changed_files::${CHANGED_FILES}"
# format the latest commit
- uses: DoozyX/clang-format-lint-action@v0.16.2
- name: Format changed files
if: steps.changed-files.outputs.any_changed == 'true'
uses: DoozyX/clang-format-lint-action@v0.17
with:
source: "."
source: "${{ steps.process-list.outputs.changed_files }}"
exclude: "extern include"
extensions: "h,cpp,c,hlsl,hlsli"
clangFormatVersion: 16
Expand Down
Loading