-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Fix poetry-install pre-commit hook configuration #10574
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
base: main
Are you sure you want to change the base?
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThe PR updates the poetry-install pre-commit hook to use system language and include default sync arguments, and enhances the documentation with a warning about prior misconfigurations and details on the default args. Flow diagram for updated poetry-install pre-commit hook configurationflowchart TD
A[".pre-commit-config.yaml"] --> B["poetry-install hook"]
B --> C["language: system"]
B --> D["args: [--sync]"]
B --> E["entry: poetry install"]
B --> F["stages: post-checkout, post-merge"]
Flow diagram for documentation update warning about poetry-install hook versionsflowchart TD
A["Documentation"] --> B["Warning for versions <= 2.2.1"]
B --> C["language: python (incorrect)"]
A --> D["Info for versions >= 2.2.2"]
D --> E["language: system"]
D --> F["args: [--sync]"]
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes - here's some feedback:
- Record the language change and default args update in the project CHANGELOG so users upgrading from earlier versions see the breaking-note.
- Consider adding a runtime check or clearer error in the hook when Poetry is not found on PATH under
language: systemto help users diagnose missing dependencies.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Record the language change and default args update in the project CHANGELOG so users upgrading from earlier versions see the breaking-note.
- Consider adding a runtime check or clearer error in the hook when Poetry is not found on PATH under `language: system` to help users diagnose missing dependencies.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes the configuration of the poetry-install pre-commit hook by changing its language setting and adding default synchronization arguments, along with updating documentation to warn users about the breaking change.
- Changed language from
pythontosystemfor the poetry-install hook - Added default
--syncargument to ensure environment synchronization with lock file - Updated documentation with version-specific warnings and argument explanations
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| .pre-commit-hooks.yaml | Updates hook configuration to use system language and adds default sync args |
| docs/pre-commit-hooks.md | Adds version warning and documents the new default sync behavior |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| entry: poetry install | ||
| language: python | ||
| language: system | ||
| args: ["--sync"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is unlucky because we deprecated --sync in favor of poetry sync.
- Change language from python to system for poetry-install hook - Add default args: ["--sync"] to poetry-install hook - Update documentation with warning about versions up to 2.2.1 Resolves: python-poetry#9393
Resolves: #9393
Summary by Sourcery
Configure the poetry-install pre-commit hook to use the system language with default sync arguments and update documentation with a warning for versions up to 2.2.1
Enhancements:
Documentation: