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

Support pre-commit to run arbitrary entrypoints even if they don't have native pre-commit support #981

Merged
merged 17 commits into from
May 29, 2023

Conversation

Spitfire1900
Copy link
Contributor

@Spitfire1900 Spitfire1900 commented May 16, 2023

  • I have added an entry to docs/changelog.md

Summary of changes

Add support for using pipx as a pre-commit hook for arbitrary python packages that do not have a pre-commit hook configured.
Some package utilities I'd like to use even but they don't have a pre-commit hook configured for them.

This allows configuration like the following to be added to a project .pre-commit-config.yaml file to run the script.

- repo: https://github.com/Spitfire1900/pipx
  rev: 9e59f26
  hooks:
  - id: pipx
    alias: yapf
    name: yapf
    args: ['yapf', '-i']
    types: ['python']

With the previous config the following command runs yapf against pipx's get-pipx.py as an example:

pre-commit run pipx --files .\get-pipx.py

Test plan

Tested by running the configuration from "Summary of changes" against my local pipx repo.

@Spitfire1900 Spitfire1900 changed the title Support pre-commit as to run any script that that has a entrypoint defined Support pre-commit to run arbitrary entrypoints even if they have native pre-commit support May 16, 2023
@Spitfire1900 Spitfire1900 changed the title Support pre-commit to run arbitrary entrypoints even if they have native pre-commit support Support pre-commit to run arbitrary entrypoints even if they don't have native pre-commit support May 16, 2023
@uranusjr
Copy link
Member

Since you basically should not use anything but pipx run, is it possible to include run in the default args so the user can (only) do args: ['yapf', '-i']?

Also it would be a good idea to add a link to pre-commit documentation in the section describing this.

@Spitfire1900
Copy link
Contributor Author

@uranusjr; I initially tried this with args: ['run'] in .pre-commit-hooks.yaml, however testing showed that args in the local .pre-commit-config.yaml overrode instead of appending to args from .pre-commit-hooks.yaml.

@Spitfire1900
Copy link
Contributor Author

I've updated the README to reference installation.doc for the pre-commit code snippet.

@Spitfire1900 Spitfire1900 force-pushed the pre-commit-hook-support branch from 7352042 to 32ab64d Compare May 18, 2023 05:04

```yaml
- repo: https://github.com/pypa/pipx
rev: 1.3.0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
rev: 1.3.0
rev: 1.2.0

Copy link
Contributor Author

@Spitfire1900 Spitfire1900 May 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pre-commit performs a git checkout against rev: and then looks for the .pre-commit-hooks.yaml file in the worktree; rev: 1.2.0 won't work as the file will be missing.

It looks like PRs are merged in pipx using the "squash" strategy, docs/installation.md will need to be updated post merge to update rev: to a valid commit hash.

@uranusjr
Copy link
Member

uranusjr commented May 25, 2023

I’m still very inclined to make this only possible to run pipx run and eliminate the run in the user command. Is it possible to get some advices from pre-commit maintainers on this?

@Spitfire1900
Copy link
Contributor Author

Spitfire1900 commented May 25, 2023

I’m still very inclined to make this only possible to run pipx run and eliminate the run in the user command. Is it possible to get some advices from pre-commit maintainers on this?

I've successfully tested using entry: pipx run in ./.pre-commit-hooks.yaml; updated PR and docs

https://pre-commit.com/#hooks-entry

require_serial is not settable in config.yaml, only hooks; so have two separate versions.
Add pipx-serial
Revert "Update installation.md"

This reverts commit 062d593.

Revert "Update .pre-commit-hooks.yaml"

This reverts commit 908c711.
@Spitfire1900 Spitfire1900 force-pushed the pre-commit-hook-support branch from 279c2f3 to be97c76 Compare May 27, 2023 03:02
Copy link
Member

@uranusjr uranusjr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be ready when CI passes.

@chrysle
Copy link
Contributor

chrysle commented May 29, 2023

All's passed!

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

Successfully merging this pull request may close these issues.

4 participants