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

poetry-export pre-commit hook doesn't fail #237

Open
mil-ad opened this issue Oct 6, 2023 · 2 comments
Open

poetry-export pre-commit hook doesn't fail #237

mil-ad opened this issue Oct 6, 2023 · 2 comments

Comments

@mil-ad
Copy link

mil-ad commented Oct 6, 2023

The poetry-expory hook is quite useful but shouldn't it return an error when it actually detects any changes? I want to use it alongside poetry-lock hook which does in fact fails after updating the lock file, and forces you to commit the changed lock file. I basically want to make sure that the poetry.lock and requirements.txt files are always in sync when commits are pushed. Right now the poetry-export does correctly update requirements.txt but doesn't block the push.

(sorry for the duplicate, I just realised poetry export has its own repo)

@xqm32
Copy link

xqm32 commented Oct 19, 2023

Are you running pre-commit run --all-files? This may not be a poetry or poetry-export problem, but a pre-commit problem.

For specific details, please refer to this code: https://github.com/pre-commit/pre-commit/blob/48f0dc9615488b583b11f2d90bd4a332701c6b6a/pre_commit/git.py#L154-L155, which means the implementation of pre-commit , a new, unstaged file will not be in all_files, so the poetry-export hook cannot detect poetry.lock. This problem does not exist if you have committed the poetry.lock file before, or if you did not execute pre-commit in the form of pre-commit run --all-files.

One possible workaround is for you to manually stage poetry.lock and execute pre-commit run --all-files again. This works for me.

@mil-ad
Copy link
Author

mil-ad commented Oct 19, 2023

My poetry.lock or requirements.txt are not new files and I'm not running with --all-files. I think the problem is in this line:

return 0

i.e. this plugin returns success unconditionally. I'd like to have the behaviour of poetry-lock hook so that if requirements.txt changes and it is not staged the command returns a non-zero error.

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

2 participants