Feature: Add poetry as pre-commit hook#2511
Feature: Add poetry as pre-commit hook#2511neersighted merged 31 commits intopython-poetry:masterfrom
Conversation
|
Thanks for the review. I fixed all issues. But before I make the draft final I want to get feedback on the topic with the chicken-egg-problem I mentioned in the comments of the connected issue. I fixed the problem with the first solution (bootstrapping) in commit 1121d06 but I would like to hear others opinions first. EDIT: Commit 1121d06 will be removed before finalization.
|
|
#2515 is solved on develop and will be released with 1.1 so I will change this PR to use develop. |
0b5eb0b to
4c3338d
Compare
|
Rebased the branch onto develop and changed merge target to develop. |
5c3a1f0 to
4c3338d
Compare
|
@sdispater Do you have any idea why the I found the same issue in your PR #2509 and the PR #2448 . The similarity is that all 3 have develop as target. But there are other PRs with target develop which succeeded in between. |
|
@Cielquan there is a bug in clikit that is being resolved. Should be fixed soon. |
|
@Cielquan please rebase this to trigger CI with the fix. |
4c3338d to
7b54c26
Compare
|
@abn Thanks for sharing. Well I updated my branch like this: $ git remote add upstream [email protected]:python-poetry/poetry.git
$ git fetch upstream
$ git checkout develop
$ git rebase
$ git checkout feature/pre-commit-hooks
$ git rebase develop
$ git push -fAll changes were fast-forward with no complication. But now the tests are totally not working 😢 The windows tests fail in the configure poetry step: |
a167b9b to
7b54c26
Compare
|
Problems solved themselves \o/ ? 🎆 |
|
You cannot have additional arguments like I don't remember why I didn't use the I guess this is fine to leave out some complexity for the user to handle for the sake of a simpler implementation in the code. |
I just tested this, errors are reported the same way, so I think the reason probably was to use |
I am fully aware of this. But I did not clarify that in the docs.
Well I use my branch with pre-commit like so: - repo: https://github.com/Cielquan/poetry
rev: 4c3338d18d319fb7edea02d653d2c65622d84ff2
hooks:
#: Check config file
- id: poetry-check
#: Update lock file
- id: poetry-lockThose two hooks did work till now without a problem, but they also don't take args. When I wrote the branch I tested it manually and did not see any errors.
Well I understand your reasoning but normally you write your EDIT: fix quote |
|
Did you run into issues installing pre-commit with poetry? On my computer it's pulling weird dependencies, I need to reproduce on a clean environment to see if I can confirm that behavior. |
This comment has been minimized.
This comment has been minimized.
|
Weird, on my computer it tries to install a bunch of stuff and fails on mysql-connector-python-rf. EDIT: Here is the pip output: Unrelated to this PR but I think if this is an actual issue it's pretty embarrassing 😉 |
This comment has been minimized.
This comment has been minimized.
|
Minor doc changes requested. I'd also love people to give the docs a close look (though I don't necessarily feel a need to hold up merging this) -- bad docs for pre-commit hooks are super common and super annoying in the ecosystem. |
Co-authored-by: Bjorn Neergaard <[email protected]>
Co-authored-by: Bjorn Neergaard <[email protected]>
Co-authored-by: Bjorn Neergaard <[email protected]>
Co-authored-by: Bjorn Neergaard <[email protected]>
Co-authored-by: Bjorn Neergaard <[email protected]>
Co-authored-by: Bjorn Neergaard <[email protected]>
Co-authored-by: Bjorn Neergaard <[email protected]>
Co-authored-by: Bjorn Neergaard <[email protected]>
Co-authored-by: Bjorn Neergaard <[email protected]>
Co-authored-by: Bjorn Neergaard <[email protected]>
Thanks for the review and suggestions. Your changed phrasing is easier to read than mine. 👍🏾 |
|
Kudos, SonarCloud Quality Gate passed! |
|
Thanks for merging 😃 |
|
Would love to use this at my repo, but it seems there is a problem at I can't find an already cached version, and the installation always fails on me, see e.g.: |
You need a paid pre-commit.ci account to access the network during the running of hooks |
Oh, I see, found some notes about this here: pre-commit-ci/issues#55 (comment) So, at GitHub level, using pre-commit.ci, this hook is unusable. This might be worth mentioning in the documents? |
|
I'm trying the # export python requirements
- repo: https://github.com/python-poetry/poetry
rev: 2fa6c17 # put here the next release with the hooks
hooks:
- id: poetry-export
files: '^.*poetry\.lock$'with the default value (i.e., |
|
For those of you who stumbled across this post like I do, you can do: |
You should also be able to shorten that snippet by the last line, because this is the default config: |
|
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. |








Pull Request Check List
Resolves: #2457
I created this draft PR for easier discussion in the issue #2457 .