Skip to content

Add prek.toml for pre-commit hook support#4855

Open
Repiteo wants to merge 3 commits into
SCons:masterfrom
Repiteo:prek
Open

Add prek.toml for pre-commit hook support#4855
Repiteo wants to merge 3 commits into
SCons:masterfrom
Repiteo:prek

Conversation

@Repiteo
Copy link
Copy Markdown
Contributor

@Repiteo Repiteo commented May 24, 2026

Implements support for prek, a tool for handling pre-commit hooks. This will open the door for us to start uniformly applying formatting changes across the repo, and ensuring they're enforced. This will be guaranteed moving forward via GitHub Actions, where prek will always be ran prior to running the various test suites

As of now, this only did a pass with some builtin checks, mainly aimed at fixing inconsistent whitespace/newlines. Excludes areas which are thirdparty/generated, or are otherwise unsuitable for formatting changes. Most notably excludes the entirety of doc/, which could eventually have portions of it added, but it's by far the area of the repo I'm least familiar with and I cannot intuit which areas are "static" versus what should be formatted (could be revisited in a future PR)

Contributor Checklist:

  • I have created a new test or updated the unit tests to cover the new/changed functionality.
  • I have updated CHANGES.txt and RELEASE.txt (and read the README.rst).
  • I have updated the appropriate documentation

@mwichmann
Copy link
Copy Markdown
Collaborator

unit tests are likely to cause some interesting problems... a number of them are more than a thousand lines long with large chunks not using what we'd consider our coding standard. If you fix a problem in there and the pre-commit wants to format that, but it's in the midst of other lines that would not line up if the changed chunk is formatted, what do you do? Are the tools smart enough for that case?

@bdbaddog
Copy link
Copy Markdown
Contributor

We're not ready to merge anything like this yet. We'll do it just after a release.
But like @mwichmann we've tried some of this before and found it often breaks some of the tests because white space such checkers remove is important to some tests.

@mwichmann
Copy link
Copy Markdown
Collaborator

we need a separate solution for "trailing whitespace in test text blocks" anyway, it's really irritating to be niggled by that. I wouldn't let that specifically block this one, fwiw.

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.

3 participants