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

Document migration path for action-based workflows that require external deps, wish to run across a matrix of platforms/python versions, or want to run manual hook types #118

Closed
CAM-Gerlach opened this issue Aug 21, 2021 · 6 comments

Comments

@CAM-Gerlach
Copy link

CAM-Gerlach commented Aug 21, 2021

Thanks for building such an awesome tool, and even more importantly such a big ecosystem around it. I couldn't find a public issue tracker for Pre-Commit.CI, so given this affects the recommended transition from this now-deprecated action to that bespoke service, I figured I'd ask here.

We're having trouble determining how to migrate from this deprecated action to pre-commit.ci, given it isn't clear how much of the critical features and functionality of the former we (r-SpaceX, Spyder, etc) currently rely on translate to the latter. In particular,

  • While best minimized to the extent possible, it is not uncommon that some hooks may require access to the current environment rather than an isolated one, require external binaries, or use a language for which pre-commit does not currently offer automated dependency installation. In our case, that includes mypy, pyanalyze and pylint, three of our core linters, which need access to the runtime environments of our packages to perform their checks, and are thus run as system hooks. With this action, we simply install the deps pinned in a requirements-dev.txt file, the same that our developers install. How would this action workflow translate to pre-commit.ci?

  • Some of our linters/checkers may inherently detect issues specific to the current Python runtime, OS or dep versions (e.g. using added/removed language features/stdlib, POSIX-only calls, portability issues, etc), and we want to ensure that all our hooks install and pass on all supported platforms. Thus, using this action, we run Pre-Commit across a matrix of OSes and Python versions. Is there any equivalent on pre-commit.ci?

  • Some hooks we only want to run in CI or locally on-demand, because they are only supported by specific platforms or are expensive. Furthermore, we sometimes want to run hooks with stages other than pre-commit in CI, or use such stages for finer-grain control over which hooks run where and when. For this, we use the extra-args feature of this action to pass the desired --hook-stage(s). Is there something comparable we're missing on pre-commit.ci, or what is the recommended approach here?

Thanks!

@asottile
Copy link
Member

asottile commented Aug 21, 2021

the issue tracker for pre-commit.ci is https://github.com/pre-commit-ci/issues -- the duplicate you're looking for is pre-commit-ci/issues#13

@CAM-Gerlach
Copy link
Author

CAM-Gerlach commented Aug 22, 2021

Thanks! I looked under the pre-commit org, where the rest of the pre-commit stuff lives, and I couldn't find any links on the pre-commit.ci site...should have just googled it, heh.

Reading the discussion over there, it looks like these capabilities currently don't exist, and if implemented, they would be paywalled. As such, while its understandable why you'd want to restrict them to avoid abuse of your infra, its a little disappointing this action is being deprecated, given a significant subset of its use cases are unsupported by pre-commit.ci, and may never be at least as far as most free and open source projects are concerned. However, if and/or when it becomes unmaintained, projects have plenty of alternatives, so long as they are aware upfront.

As such, it might be a good idea to at least mention in the Readme that pre-commit.ci isn't a full replacement this action for some current use cases, and suggest a recommended alternative (adopt it, fork it, use a different action, call it manually, etc).

@asottile
Copy link
Member

nah I don't think I will, language: system is already the unsupported escape hatch

@CAM-Gerlach
Copy link
Author

I see; considering that this excludes popular tools like pylint and mypy that many projects are likely currently using, and which the pre-commit docs suggest using system for, I'd think it would be at least worth a mention, but that's your call of course. Regardless, thanks again for the hard work you put in to pre-commit and this action.

@asottile
Copy link
Member

the pre-commit docs do not suggest using system, they show how to use system but it is not a suggested or supported path

@CAM-Gerlach
Copy link
Author

Thanks for the clarification. Is duplicating the project's deps in additional_dependencies the recommended way of handling this (seems like this is what you suggest for MyPy), or are tools that interact with the project env officially incompatible by design with pre-commit, as your oft-referenced advice on pylint seems to indicate?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants