That's what I was worried about too. If the bot pushes a commit, then the PR author won't be able to push any additional commits unless they either pull first or force push. Personally I would find that a little surprising, and might not even notice it when I do "git push". Plus I feel like this would push people into the bad habit of always force pushing.
Aaron Meurer On Tue, Mar 28, 2023 at 9:50 AM Jason Moore <[email protected]> wrote: > 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 > <https://groups.google.com/d/msgid/sympy/CAP7f1Ah9-uRCPqpFWzbNLp_4z4gBSmQ6xmr0_aHPcuD78W6%3DZA%40mail.gmail.com?utm_medium=email&utm_source=footer> > . > -- 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/CAKgW%3D6%2BUYTFZWf%3DHJJZPx0UukKx1PgnTtY8%3DcK7MwJOTOmD1Tw%40mail.gmail.com.
