Skip to content

Conversation

@Skylion007
Copy link
Contributor

Summary

Test Plan

@MichaReiser
Copy link
Member

This is interesting. How fast is this pre-commit step? I'm a bit annoyed by how slow our pre-commit is if I use --all-files (I have to use --all-files because I never run pre-commit on individual commits, we aren't friends ;))

I guess my concern is, I don't want to make my workflow significantly slower and this is already something that otherwise gets catched in CI

@AlexWaygood
Copy link
Member

I'm also a bit sceptical this will be that useful -- it's a pretty fast hook IIRC, but I'm not convinced it'll be that useful given that merge conflicts are pretty easy to catch in CI through our existing checks. And I agree that our pre-commit is already somewhat slow locally.

@Skylion007
Copy link
Contributor Author

In my experience, it's a super fast lightweight hook; and one of the standard pre-commit hooks. The builtin hooks are pretty optimized.

This also catches merge conflicts in code areas that are not run: like say documentation, text files or other file types.

@AlexWaygood
Copy link
Member

AlexWaygood commented Apr 10, 2025

Okay, I checked out this PR locally and the new hook is basically instantaneous on this repo. So if this will help some of our contributors, let's go for it.

Thanks!

@AlexWaygood AlexWaygood merged commit 06ffeb2 into astral-sh:main Apr 10, 2025
22 checks passed
@AlexWaygood AlexWaygood added the ci Related to internal CI tooling label Apr 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci Related to internal CI tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants