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

satisfy CI clang-format #339

Merged
merged 2 commits into from
Jan 6, 2025
Merged

satisfy CI clang-format #339

merged 2 commits into from
Jan 6, 2025

Conversation

Dewb
Copy link
Contributor

@Dewb Dewb commented Jan 5, 2025

What does this PR do?

fixes format warning in CI introduced in #336

Provide links to any related discussion on lines.

n/a

How should this be manually tested?

n/a

Any background context you want to provide?

Looks like local clang-format results and CI results are not consistent, c-f didn't complain for me locally. (Apple clang 15.0.0, latest git clang-format integration via homebrew.)

If the related Github issues aren't referenced in your commits, please link to them here.

I have,

  • (n/a) updated CHANGELOG.md and whats_new.md
  • (n/a) updated the documentation
  • (n/a) updated help_mode.c (if applicable)

@Dewb
Copy link
Contributor Author

Dewb commented Jan 5, 2025

Ah, I got to the bottom of why I did not catch this locally; it was user error. Running git-clang-format via make format works as intended if you do it on uncommitted/staged changes. I was under the impression that if I forgot to format before submitting, I could add a blank line to the relevant files and run it again. For some reason that doesn't work; to get proper formatting in this case, you need to supply the branch or commit ID you want to compare against.

If I (the person who introduced the CI formatting requirement) can't figure out how to do this correctly then it's probably not workable for others. I've now also added the "main" argument to the git-clang-format command in the Makefile, which might complicate some edge cases, but in general it should mean make format works in the expected way and formats everything that's new in a PR branch, whether it's been committed or not.

@tehn tehn merged commit 937b002 into monome:main Jan 6, 2025
6 checks passed
@Dewb Dewb deleted the fix-format branch January 7, 2025 02:59
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

Successfully merging this pull request may close these issues.

2 participants