-
Notifications
You must be signed in to change notification settings - Fork 153
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
chore: pre-commit #8
Conversation
7501401
to
5d92f75
Compare
(Sorry for the big delay -- I still plan to look into this but haven't gotten to it yet) |
Take your time. I can rebase whenever you need me to. Would you be interested in enabling pre-commit.ci on the three example repos in pybind11? I think it was enabled for pybind11 itself, but it's not on the example repos, and that would be very nice to keep up to date and correct user contributions automatically. |
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
b3b50e3
to
089e4a1
Compare
Rebased. |
b6837e0
to
13dbff1
Compare
1c17434
to
a2b8b20
Compare
42ce5e7
to
0106656
Compare
c41d953
to
f935f93
Compare
4240a97
to
e1cb670
Compare
56d7e93
to
e80edb1
Compare
c30294a
to
af57451
Compare
d7117a4
to
983d6c0
Compare
Hi Henry, this PR has been up for a long time, and you even rebased it once to keep track of changes. I appreciate this very much. I also played with pre-commit to see how I might integrate it. Altogether, I totally get why you created this PR and find it a good default for some community projects. (And in general, the guidelines that you provide for other open source projects are a great start). But what can I say ... I just don't like it :-(. It find it adds too much "process" for what is at the end of the day my hobby project. I want a zero-friction way to concentrate the few minutes I have sometimes to work on this project, and this adds one rather complicated piece of technology that would now lie between me and the github repository. So rather than leaving this PR open for another 2 years, I will close it to reflect the reality, which is that I don't really want pre-commit for this repo. I think that the total amount of time it will save me is less than the amount of time I will need to invest to fully understand it, keep it running, fix flakes, and get my own changes to conform to its wishes. Wenzel |
I sadly was about to update this PR, I've found GitHub's "every PR you have open list", and have slowly been working though it. (Actually I had fun writing a GraphQL query that produced the list before finding out it's basically a button in GitHub.) FYI, this has become quite a bit cleaner and simpler in the last year or two with ruff taking over most of the smaller projects and doing a lot with just a single very fast binary. You don't have to add checks you don't want to mess with, but having the ability to add arbitrary checks, written in over a dozen languages, including ones you write yourself, is incredibly powerful. It's also something you can run in CI or locally, unlike just adding lots of custom runs of things in CI. For example, for Another one I've used that is incredibly powerful is auto file regeneration. Using a tool like cog or a custom Python script, you can ensure that a generated file is always regenerated from its source. For example, the Scientific Python Developer's Guide uses it to regenerate the code examples in the pages to be the output of the cookiecutter template it also provides. I've also used it to make sure a requirements.txt file was regenerated from a pdm lock file. I believe every repo I am a maintainer on (over 40) uses pre-commit, along with basically all my personal ones. Over 63K GitHub repos use it, including CPython itself. Just knowing it is a useful skill. I would treat it like a test suite that runs really quickly, but was designed to check static things and supports checks that change things. Even if I'm the only committer, it removes many ways to make a mistake - the Schema check has saved me from pushing broken GitHub Actions workflows. Validate-pyproject catches errors in pyprojects - I have downloaded every pyproject.toml file from PyPI, and there are a lot of errors for anything that isn't checked at the 1-5% level. You don't have to "install" if you don't want to, and just run it directly (I do that most of the time), or even just run it in CI. If you don't push directly and always make PRs, you can even have pre-commit.ci run and auto-push fixes to PRs. It's like having a helpful coworker that always cleans up your work a few seconds after you do a quick and sloppy push. If you don't like formatters, don't add formatters to your pre-commit file. But linters and validators are still really useful. I'll not make another PR adding pre-commit, then. Just wanted to dump my thoughts; pre-commit is the one tool I use everywhere and rely on more than any other. (It also frustrates me that the author is also the author of Flake8 and hates the Astral team for making Ruff and will never add uv support; I'd love for the setup phase of pre-commit for Python extensions to be powered by uv, as it basically makes all pip installs take 0 seconds.) I'll keep in mind that you consider nanobind a "hobby project", good to know if people ask which one to use. :P |
Adding pre-commit. I did not add black yet, though I'd highly recommend it; given the desire to avoid clang-format, I wasn't sure if black would be accepted.
Followup to #4.
I would highly recommend trying out using pre-commit locally. It is really an amazing developer tool. You can see my recent slides on it here.
I didn't turn on testing for it - I'd recommend using pre-commit.ci, as it will auto-fix PRs too, and keep the pins up to date; but if you'd rather GHA, I can add that.