Hi all,
There are two open PRs discussing the potential use of pre-commit and
pre-commit.ci in SymPy:
https://github.com/sympy/sympy/pull/24908
https://github.com/sympy/sympy/pull/24941
I want to know what others think specifically about enabling
pre-commit.ci on the sympy repo or otherwise encouraging use of
pre-commit for contributors.
I'll explain below but you can also read about pre-commit and
pre-commit.ci here:
https://pre-commit.com/
https://pre-commit.ci/
The pre-commit tool is something that can be installed locally and can
be used directly or as a git hook so that when making a git commit
some quick checks can run on the code. The PR gh-24908 would add some
configuration for this so that a contributor can either run some
checks before committing or can install pre-commit as a git hook so
that git commit automatically runs the checks. The configuration in
gh-24908 means that pre-commit runs flake8 and ruff but specifically
only on the files that are being changed in the commit which is
convenient because it is much faster than checking the whole codebase.
To be clear adding the pre-commit config to the sympy repo does not
make it mandatory for all contributors to use the git hook. However it
could be something that is "recommended" as it will quickly show up
some common problems that would otherwise fail the checks in CI after
opening a PR or after pushing to a PR.
What is also discussed in those PRs is adding
pre-commit.ci to the
sympy repo which is something different from just adding a pre-commit
configuration that contributors can choose to use or not. The
difference is that
pre-commit.ci is a GitHub bot that will run the
pre-commit hooks on all pull requests and can often fix the problems
automatically by pushing a new commit to the PR.
Currently if someone pushes a PR that has simple problems like
trailing whitespace or unnecessary imports then the flake8 or quality
checks in CI will report an error asking the contributor to fix those
problems. With
pre-commit.ci we could make it so that those problems
are just fixed automatically without the contributor needing to do
anything.
Both trailing whitespace and unnecessary imports are automatically
fixable e.g. there is already a bin/strip_whitespace script and ruff
can fix the imports with:
ruff check --select F401 --fix sympy
Obviously other things could be fixed automatically but these are the
two that I see most often where someone pushes and then needs to push
a followup fixing commit after seeing CI checks fail. If
precommit.ci
was used there would be no need to push a follow up commit because the
bot would just do it automatically.
On the other hand if someone uses the pre-commit hook locally then
that could fix these things automatically before pushing and there
wouldn't be any need for the bot to fix them in CI. The advantage of
the CI bot would be that it could apply simple fixes for someone who
does not use the git hook and didn't check pre-commit before pushing.
To be clear there would not be any requirement for any individual
contributor to use pre-commit locally. However if
pre-commit.ci runs
on PRs then that is obviously not optional and there would be a bot
pushing fix commits to PRs.
Does anyone have any thoughts on enabling
pre-commit.ci or otherwise
encouraging contributors to use pre-commit?
--
Oscar