Skip to content

Add minimal pre-commit config#406

Merged
knoepfel merged 9 commits intoFramework-R-D:mainfrom
olantwin:pre-commit
Mar 13, 2026
Merged

Add minimal pre-commit config#406
knoepfel merged 9 commits intoFramework-R-D:mainfrom
olantwin:pre-commit

Conversation

@olantwin
Copy link
Contributor

Implement #390 .

For now, only local hooks (no CI config).

For now ruff, clang-format and some of the hooks included with pre-commit.

I'll add cmake-format and jsonnet-fmt next.

@olantwin
Copy link
Contributor Author

@phlexbot yaml-fix

1 similar comment
@knoepfel
Copy link
Member

@phlexbot yaml-fix

@github-actions
Copy link
Contributor

Automatic YAML formatter fixes pushed (commit 6ac2d04).
⚠️ Note: Some issues may require manual review and fixing.

@knoepfel knoepfel linked an issue Mar 10, 2026 that may be closed by this pull request
@knoepfel
Copy link
Member

@olantwin, apologies about the YAML format problems. The changes in .github/workflows are not formally related to this PR.

@greenc-FNAL
Copy link
Contributor

Note: we use gersemi in place of cmake-format.

@olantwin
Copy link
Contributor Author

Note: we use gersemi in place of cmake-format.

Thanks for the pointer! Added.

@olantwin
Copy link
Contributor Author

Rebased and removed the spurious changes in the .github CI workflows introduced by the yaml bot.

@knoepfel
Copy link
Member

@olantwin, thank you for this PR! It will simplify our development workflow in the way we had hoped. I've been testing it locally and encountered a few oddities. I'm not sure they have anything to do with the PR itself or something else...so I'll get back to you on that.

Thank you for patience.

The files in that directory import files that are generated during the build stage.
They cannot be meaningfully linted during a pre-commit step.
@codecov
Copy link

codecov bot commented Mar 13, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

@@            Coverage Diff             @@
##             main     #406      +/-   ##
==========================================
+ Coverage   83.71%   84.25%   +0.54%     
==========================================
  Files         126      128       +2     
  Lines        3291     3322      +31     
  Branches      583      564      -19     
==========================================
+ Hits         2755     2799      +44     
+ Misses        325      324       -1     
+ Partials      211      199      -12     
Flag Coverage Δ
unittests 84.25% <ø> (+0.54%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 7 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ba9cba9...a40cced. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@knoepfel
Copy link
Member

knoepfel commented Mar 13, 2026

@olantwin, I pushed some changes to the configuration which, I believe, now brings the PR in line with GitHub actions. After this PR is merged, I will create another PR that applies the changes from pre-commit run --all-files.

@olantwin
Copy link
Contributor Author

No problem, hope it's going to be useful!

Copy link
Contributor

@greenc-FNAL greenc-FNAL left a comment

Choose a reason for hiding this comment

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

Any chance of an addition to DEVELOPING.md describing how to actually activate these hooks locally?

@olantwin
Copy link
Contributor Author

@greenc-FNAL Thanks for the feedback! I've added a few lines explaining the basic usage to DEVELOPING.md and updated the clang-format version.

@olantwin olantwin requested a review from greenc-FNAL March 13, 2026 17:21
@olantwin
Copy link
Contributor Author

@phlexbot markdown-fix

@knoepfel knoepfel dismissed greenc-FNAL’s stale review March 13, 2026 17:29

Changes made by @olantwin in most recent commit

Copy link
Member

@knoepfel knoepfel left a comment

Choose a reason for hiding this comment

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

We'll deal with the markdown error in a separate PR. Right now, @phlexbot responds only to specific individuals (or individuals on a dedicated team...I can't remember which).

@knoepfel knoepfel merged commit f7817d3 into Framework-R-D:main Mar 13, 2026
60 of 61 checks passed
@knoepfel
Copy link
Member

Thanks for this @olantwin!

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.

Using pre-commit hooks

3 participants