I like the idea of the following With https://github.com/sympy/sympy/pull/24908 you can pip install pre-commit and then any time you want to run the precommit checks you just run:
$ pre-commit flake8...................................................................Passed ruff.....................................................................Passed I especially like that they will only check changed files. /c On Friday, March 31, 2023 at 9:20:57 AM UTC-5 Oscar wrote: > Okay, so it seems people are not keen on integrating the pre-commit.ci > bot. > > We can of course continue to make a pre-commit config that any > contributor wants to use. > > Another question I have is what people think about using dependabot. > We regularly have problems where new versions of SymPy's dependencies > cause breakage in CI. This can happen because of things like: > > - New Sphinx versions. Basically every significant release of Sphinx > seems to break some part of building SymPy's docs. > - New numpy/scipy etc versions. It is common that these will introduce > things like deprecation warnings and also various things will break > because of the removal of functions etc. > - New versions of linters like flake8 and its plugins or mypy. > > Whenever this happens and CI gets broken it is both disruptive and > confusing for any contributors whose PRs will fail CI checks because > of problems that are unrelated to the changes they have made. > > A common way to prevent CI from breaking is to pin dependencies of the > things that are used in CI but the difficulty with that is the need to > keep the pinned versions updated as new releases are made. A widely > used solution for this is dependabot which is a bot that can > automatically open PRs to update pinned versions. Depending on how > many versions are pinned and how often the pinned things make new > releases there would be some number of PRs from dependabot updating > versions one dependency at a time (dependabot does not have a way to > do several dependencies in a single PR). > > The advantage of using dependabot is that if a new version of some > dependency causes CI to break then it will only break in dependabot's > PR that tries to bring in the update. Otherwise we can keep things > updated by updating the versions only when CI passes. > > The dependabot update PRs can be annoying but at the same time they > are very simple PRs that are easy to review and merge if CI passes. > When CI fails they would often also be easy problems to fix and would > often make suitable "easy to fix" issues (we can afford to do this if > there is no need to rush in a fix because of broken CI). > > Does anyone have any views on enabling dependabot on the SymPy repo > (or some similar alternative)? > > -- > Oscar > > On Tue, 28 Mar 2023 at 22:53, Aaron Meurer <asme...@gmail.com> wrote: > > > > 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 <moore...@gmail.com> 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 <(530)%20601-9791> > >> > >> > >> On Tue, Mar 28, 2023 at 3:41 PM Oscar Benjamin <oscar.j....@gmail.com> > 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 sympy+un...@googlegroups.com. > >>> 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 sympy+un...@googlegroups.com. > >> To view this discussion on the web visit > https://groups.google.com/d/msgid/sympy/CAP7f1Ah9-uRCPqpFWzbNLp_4z4gBSmQ6xmr0_aHPcuD78W6%3DZA%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 sympy+un...@googlegroups.com. > > To view this discussion on the web visit > https://groups.google.com/d/msgid/sympy/CAKgW%3D6%2BUYTFZWf%3DHJJZPx0UukKx1PgnTtY8%3DcK7MwJOTOmD1Tw%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 sympy+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/sympy/27aeee9f-4448-470e-af01-0ef358ff2e56n%40googlegroups.com.
