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

Different Formatting Results Between Action and Locally #198

Open
DeveloperPaul123 opened this issue May 22, 2024 · 5 comments
Open

Different Formatting Results Between Action and Locally #198

DeveloperPaul123 opened this issue May 22, 2024 · 5 comments

Comments

@DeveloperPaul123
Copy link

DeveloperPaul123 commented May 22, 2024

We are using clang-format 18 via LLVM 18.1.5 but after formatting locally the files that the CI complains about in its checks, no changes are applied. Is there something we're missing? Below is how we are using the action:

- name: Check clang-format
    uses: jidicula/[email protected]
    with:
      clang-format-version: 18
      exclude-regex: '(3rdparty|libs|scripts|test|tests)'
@DeveloperPaul123
Copy link
Author

We have a .clang-format and .clang-format-ignore in the root of the project.

@jidicula
Copy link
Owner

Interesting! Thanks for submitting this issue.

Have you noticed any overlap with the files it's complaining about in CI and your .clang-format-ignore file? Otherwise, my other suspicion would be that the exclude-regex is not correct.

Finally, is there a difference between the minor and/or patch versions of clang-format running in your CI as opposed to running locally? As of 0391c32 (haven't included this in a release yet), I've added a clang-format --version output at the beginning of this action's execution. You can try this out by opening a PR in your repo with that workflow job step you included modified to:

- name: Check clang-format
    uses: jidicula/clang-format-action@0391c32fa54dcffc9d61e34f106433ab053c20fc
    with:
      clang-format-version: 18
      exclude-regex: '(3rdparty|libs|scripts|test|tests)'

just to see what's output. If you do notice a difference in versions and you see a difference in behaviour, please leave a comment (and vote) in #192.

@DeveloperPaul123
Copy link
Author

Interesting...

Just did a test with clang-format 18.1.3 (this was what was reported on the CI when I used the commit you mentioned in a new PR) and there are in fact differences with how formatting is handled between 18.1.3 and 18.1.5/18.1.6. I'll be sure to vote on the linked discussion as well.

@jidicula
Copy link
Owner

That is very unfortunate and frustrating that LLVM included breaking changes in a patch version increment - I had been hoping there wouldn't be any impact for this shape of change.

I've opened #199 to look into a better solution for this.

In the meantime, I suppose your only mitigation is to upgrade your toolchain (probably annoying reactive work) to match what's in CI.

I'm also going to cut a new release right away so users have some more visibility into what specific clang-format version is executed by this action.

@DeveloperPaul123
Copy link
Author

That is very unfortunate and frustrating that LLVM included breaking changes in a patch version increment - I had been hoping there wouldn't be any impact for this shape of change.

I agree but oh well. We'll figure out a workaround. Supporting system clang-format would definitely work for us. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants