Skip to content

Comments

Fix the files pattern in Poetry pre-commit hooks#4907

Merged
neersighted merged 1 commit intopython-poetry:masterfrom
floatingpurr:fix/pre-commit-hook
Jan 17, 2022
Merged

Fix the files pattern in Poetry pre-commit hooks#4907
neersighted merged 1 commit intopython-poetry:masterfrom
floatingpurr:fix/pre-commit-hook

Conversation

@floatingpurr
Copy link
Contributor

@floatingpurr floatingpurr commented Dec 17, 2021 •
edited
Loading

Pull Request Check List

Resolves: #2511 (comment)

Affected hooks:

  • poetry-check
  • poetry-export
  • Added tests for changed code. not necessary
  • Updated documentation for changed code. already present

What a surprise. Poetry-based pre-commit hooks are going to be backed into Poetry itself! 🎉

I've just given them a spin this way:

# export python requirements
- repo: https://github.com/python-poetry/poetry
  rev: 2fa6c17 # put here the next release with the hooks
  hooks:
    - id: poetry-export

but it didn't work. The output is:

$ pre-commit run poetry-export --files poetry.lock
poetry-export........................................(no files to check)Skipped

It looks like poetry.lock doesn't match the files pattern and skips the files.
This quick PR fix this problem.

Tests pointing at my PR with pre-commit 2.16.0 on a *nix enviroment:

repos:
  - repo: https://github.com/floatingpurr/poetry
    rev: 26651c8
    hooks:
      - id: poetry-export

  - repo: https://github.com/floatingpurr/poetry
    rev: 26651c8
    hooks:
      - id: poetry-check
$ pre-commit run poetry-export --files poetry.lock
poetry-export............................................................Passed
$ pre-commit run poetry-check --files pyproject.toml
poetry-check.............................................................Passed

Hope this helps

Cielquan
Cielquan previously approved these changes Jan 3, 2022
View reviewed changes
@Cielquan
Copy link
Contributor

Cielquan commented Jan 3, 2022 •
edited
Loading

I tested this myself and can confirm the issue and that the fix is working.

As the author of the PR #2511 which introduced this I dunno why this went undetected.

EDIT: pre-commit version 2.13 on linux

@floatingpurr
Copy link
Contributor Author

floatingpurr commented Jan 3, 2022

Thank you for your check.

neersighted
neersighted previously approved these changes Jan 17, 2022
View reviewed changes
@neersighted neersighted reopened this Jan 17, 2022
@neersighted neersighted dismissed stale reviews from Cielquan and themself via da632ad January 17, 2022 15:38
@floatingpurr
Copy link
Contributor Author

floatingpurr commented Jan 17, 2022 •
edited
Loading

Why was black failing on ./src/poetry/utils/env.py?

Edit: probably this PR has been rebased. Thanx

@neersighted
Copy link
Member

neersighted commented Jan 17, 2022

A quirk of pre-commit -- an earlier PR did not touch src/poetry/utils/env.py, but did update the black/pre-commit version. Because of that, the PR was successfully merged into master but started failing once pre-commit did a run against all (instead of changed) files.

Might be a bug in pre-commit.ci (not sure if it's expected or not), but it's certainly an edge case.

@neersighted neersighted merged commit a358be7 into python-poetry:master Jan 17, 2022
@bentocin bentocin mentioned this pull request Jul 17, 2023
Merged
2 tasks
@github-actions
Copy link

github-actions bot commented Feb 29, 2024

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@neersighted neersighted neersighted approved these changes

+1 more reviewer

@Cielquan Cielquan Cielquan left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@floatingpurr @Cielquan @neersighted