Skip to content

fix(add): include pyproject.toml path in "already present" message#10908

Open
SarthakB11 wants to merge 2 commits into
python-poetry:mainfrom
SarthakB11:fix/issue-10179
Open

fix(add): include pyproject.toml path in "already present" message#10908
SarthakB11 wants to merge 2 commits into
python-poetry:mainfrom
SarthakB11:fix/issue-10179

Conversation

@SarthakB11
Copy link
Copy Markdown
Contributor

Pull Request Check List

Resolves: #10179

  • Added tests for changed code.
  • Updated documentation for changed code.

The "already present in the pyproject.toml and will be skipped" notice doesn't say which pyproject.toml. With poetry self add, a poetry-managed project next to your own, or any non-cwd invocation, you have to go grep for the file.

notify_about_existing_packages now includes self.poetry.file.path in the message, styled with the same <c2> markup the add command uses elsewhere for paths. The two assertions in tests/console/commands/test_add.py were updated to interpolate the fixture's path.

Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey - I've left some high level feedback:

  • In notify_about_existing_packages, consider avoiding the two adjacent f-strings and instead build the message as a single f-string or via .format() to improve readability and make the linter’s job easier.
  • The tests interpolate app.poetry.file.path, which may be an absolute path; if the CLI output is intended to show a path relative to the current working directory, you might want to normalize it in the command code (and reflect that in the assertions) for more user-friendly output.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `notify_about_existing_packages`, consider avoiding the two adjacent f-strings and instead build the message as a single f-string or via `.format()` to improve readability and make the linter’s job easier.
- The tests interpolate `app.poetry.file.path`, which may be an absolute path; if the CLI output is intended to show a path relative to the current working directory, you might want to normalize it in the command code (and reflect that in the assertions) for more user-friendly output.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

… path

Same message change as the main add command; this test was missed.
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.

Error message from poetry self add referencing pyproject.toml

1 participant