Skip to content
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

pin test versions in pre-commit #187

Closed
jakkdl opened this issue Apr 30, 2023 · 4 comments
Closed

pin test versions in pre-commit #187

jakkdl opened this issue Apr 30, 2023 · 4 comments

Comments

@jakkdl
Copy link
Member

jakkdl commented Apr 30, 2023

I have not evaluated alternatives or looked at shed's pip-tools setup #182 (comment), just making an issue to track it for now

@jakkdl
Copy link
Member Author

jakkdl commented May 19, 2023

By default versions are pinned in pre-commit, but I had set env PYRIGHT_PYTHON_FORCE_VERSION=latest - probably just because I was irritated with the warning that there was a new version available (turns out there's a PYRIGHT_PYTHON_IGNORE_WARNINGS i can use 😅). So the immediate problem should be solvable merely by not setting that environment variable - but I would like to setup auto-updates now that development has slowed down so stuff like updates to flake8 doesn't catch us off guard again. And auto-updating the linter versions in pre-commit is nice too.

shed's pip-tools setup doesn't integrate with pre-commit, and the requirements.[txt/ini] file in trio has made me gone mad - though my frustrations with it could probably be addressed by running it from tox with a specified python version (which I notice shed doesn't, so tox -e deps on non-python3.11 will lead to changes).

For pre-commit, options include:

  • pre-commit.ci bot
    • pro:
      • CI checks being faster as it caches versions.
      • autofix pr's
    • cons:
      • to see output you have to go to pre-commit's site (I don't personally experience this as much of a con, but you mentioned it previously)
      • limited configuration:
        • cannot setup auto-merge on non-breaking changes
        • updates limited to weekly/monthly/quarterly
    • While pyright can't run inside the pre-commit CI bot (since that lacks internet access, but we can trigger it manually from ci.yml), it'll still see version updates and trigger a PR - in which the GH action will run pyright as usual
  • GH action that runs pre-commit autoupdate https://browniebroke.com/blog/gh-action-pre-commit-autoupdate/
    • pro/cons are just the reverse of the above

I'd personally like to try out pre-commit.ci to see how that works. So for that you need to sign in with github and add it at https://pre-commit.ci/ - and after that I'll adjust ci.yml so the GH action only runs pyright, and doesn't duplicate all of pre-commit.

For direct dependencies

it's either dependabot or custom GH actions. This likely will be triggered much less often with only two dependencies that can update, and seeing dependabot everywhere I'd like to familiarize myself with it. Setting it up should be fairly trivial: https://til.simonwillison.net/github/dependabot-python-setup - I can push a PR with the yaml file, but after that you'd need to push the buttons in the settings.

pyright GH-action

While reading up on stuff I also stumbled upon https://github.com/microsoft/pyright/blob/main/docs/ci-integration.md#running-pyright-as-a-github-action https://github.com/jakebailey/pyright-action - I tried it out in a separate repo, and while it might be superior for some reason or another it would require specifying typing dependencies some place other than in .pre-commit-config.yaml. Maybe for another time.

@jakkdl
Copy link
Member Author

jakkdl commented May 20, 2023

ah, reading up more about direct dependency pinning - it sounds like it's a bad idea to pin those as it'll very possibly lead to dependency conflicts. But one can specify the package dependencies in the testing requirements, so updates to them would trigger a check.
flake8 is pinned in pre-commit, but pinning libcst in there is not really viable, so I suppose one would do that with a test_requirements.txt... 🤔

Interesting stuff, but I don't think it's worth putting time into that for flake8-trio - esp if we setup regular updates for pre-commit, which will as a side-effect pick up breaking libcst updates when it runs on updated linters.

@jakkdl
Copy link
Member Author

jakkdl commented May 30, 2023

@Zac-HD would you mind adding https://pre-commit.ci/ to the repo to get automatic updates?

@jakkdl
Copy link
Member Author

jakkdl commented Mar 14, 2024

We now have pre-commit running regularly, so pinning shouldn't be necessary 🚀

@jakkdl jakkdl closed this as completed Mar 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant