I personally would find a bot adding commits to my work a bit intrusive. If the bot posted a comment to the issue telling me what to fix, that would be preferable. Right now we have to make a few clicks to see why the linter failed.
Conda forge has a bot that will add commits to your branch, but only if you explicitly ask it to. If we had some bot commands like '@sympy/bot please fix flake8 issues' then that would run the fix and add the commit, but it is the author's choice to do so. Jason moorepants.info +01 530-601-9791 On Tue, Mar 28, 2023 at 3:41 PM Oscar Benjamin <[email protected]> wrote: > 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 > > -- > You received this message because you are subscribed to the Google Groups > "sympy" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to [email protected]. > To view this discussion on the web visit > https://groups.google.com/d/msgid/sympy/CAHVvXxSmJ2aGsiHf6%2BNFLaBL0xkUeH0_CJ86uqQR-py6uCZnxg%40mail.gmail.com > . > -- You received this message because you are subscribed to the Google Groups "sympy" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/sympy/CAP7f1Ah9-uRCPqpFWzbNLp_4z4gBSmQ6xmr0_aHPcuD78W6%3DZA%40mail.gmail.com.
