Skip to content

CI: Post pre-commit fixes as suggestions#6809

Merged
echoix merged 10 commits intoOSGeo:mainfrom
echoix:pre-commit-suggestions
Feb 1, 2026
Merged

CI: Post pre-commit fixes as suggestions#6809
echoix merged 10 commits intoOSGeo:mainfrom
echoix:pre-commit-suggestions

Conversation

@echoix
Copy link
Copy Markdown
Member

@echoix echoix commented Dec 23, 2025

In order to simplify updating clang-format versions, I want to remove the clang-format workflow, and only use pre-commit for handling clang-format.
This means that the pre-commit run here should become a required check, and the clang-format workflow should not be a required check anymore. After merging this, these two changes should be made. cc @wenzeslaus

Until clang-format is removed, duplicate suggestions might be posted for clang-format fixes (one from clang-format workflow, and one from pre-commit). If you prefer, I can include the removal here, but the changes to the required checks must be done right before merging this (otherwise the existing and required clang-format check won't report a success).

To make this possible, I added a new tool source and new workflow source for posting PR suggestions. It is not intended to remove the older sources yet, as it allows un-updated PRs and branches to still have the suggestions posted.

I also needed to use prek instead of pre-commit (I've been using it since fall). https://github.com/j178/prek https://prek.j178.dev/ It is a rust equivalent of pre-commit, similar to ruff/uv, and installs way faster, and is faster in some tasks. It allows to skip hooks, rather than including hooks. That allows for removing running hooks that already post suggestions or handled in other workflows, collect the changed files for suggestions, and then run all pre-commit hooks without posting suggestions.

PR #6808 is intended to add fixes from an additional tool.

I also had to remove the created core_modules_with_last_commit.json file, as that changed file was creating problems when checking for changed files (to upload for suggestions). So, before deleting it, I uploaded it as an artifact.

@echoix echoix requested a review from wenzeslaus December 23, 2025 15:25
@github-actions github-actions Bot added the CI Continuous integration label Dec 23, 2025
@echoix echoix enabled auto-merge (squash) December 23, 2025 18:48
@echoix echoix disabled auto-merge December 23, 2025 18:48
@echoix
Copy link
Copy Markdown
Member Author

echoix commented Jan 9, 2026

@wenzeslaus Would you mind taking a look at this in the following days?

@echoix
Copy link
Copy Markdown
Member Author

echoix commented Jan 31, 2026

I still want this to unblock my other work to finish upgrading clang-format to the latest point release that fixes some bad parsing/formatting issues. Also want it before 8.5.

Copy link
Copy Markdown
Member

@wenzeslaus wenzeslaus left a comment

Choose a reason for hiding this comment

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

More pre-commit replacing specific checks is good since we are using pre-commit anyway. A lot of changes, but they make sense.

BTW, CPython lint workflow us only a prek action.

@echoix
Copy link
Copy Markdown
Member Author

echoix commented Feb 1, 2026

I think they changed since I wrote the PR, I saw in some newer peek release notes the mention.

I'll merge, and then tell me when you make this required and the clang-format not required @wenzeslaus

@echoix echoix merged commit 2d18639 into OSGeo:main Feb 1, 2026
28 checks passed
@echoix echoix deleted the pre-commit-suggestions branch February 1, 2026 17:01
@github-actions github-actions Bot added this to the 8.5.0 milestone Feb 1, 2026
@wenzeslaus
Copy link
Copy Markdown
Member

Additional checks are now required, while Formatting check is not. Let me know if that works as expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI Continuous integration

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants