-
Notifications
You must be signed in to change notification settings - Fork 2.4k
feat: add an option to generate a project with package mode = false #10469
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 GuideAdds a new CLI flag to disable package mode during project initialization, propagates the flag through the init command into the layout generator, and updates the layout to skip optional fields and include package-mode metadata, accompanied by new tests. Sequence diagram for project initialization with package mode optionsequenceDiagram
actor User
participant CLI as Poetry CLI
participant InitCommand
participant Layout
User->>CLI: poetry init -N
CLI->>InitCommand: parse options (disable-package-mode)
InitCommand->>Layout: __init__(..., package_mode=False)
InitCommand->>Layout: generate_project_content()
Layout-->>InitCommand: project content (skipping optional fields)
InitCommand-->>CLI: write pyproject.toml
Class diagram for InitCommand and Layout changesclassDiagram
class InitCommand {
+_init_pyproject(...)
+option("disable-package-mode", "-N", ...)
}
class Layout {
+__init__(..., package_mode: bool = True)
+generate_project_content() TOMLDocument
-_package_mode: bool
}
InitCommand --> Layout : uses
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 @danielepintore - I've reviewed your changes - here's some feedback:
- Consider adding tests to verify that explicitly provided metadata (e.g. version, description, authors) is still preserved when
--disable-package-mode
is used, not just the removal of defaults. - It might be clearer to consolidate the logic that removes optional fields in
generate_project_content
into a single helper or loop to reduce duplication and improve maintainability. - Extracting
POETRY_DEFAULT_AUTHOR
into a shared defaults module (rather than hardcoding it inlayout.py
) could help keep default values consistent across the codebase.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider adding tests to verify that explicitly provided metadata (e.g. version, description, authors) is still preserved when `--disable-package-mode` is used, not just the removal of defaults.
- It might be clearer to consolidate the logic that removes optional fields in `generate_project_content` into a single helper or loop to reduce duplication and improve maintainability.
- Extracting `POETRY_DEFAULT_AUTHOR` into a shared defaults module (rather than hardcoding it in `layout.py`) could help keep default values consistent across the codebase.
## Individual Comments
### Comment 1
<location> `src/poetry/console/commands/init.py:146` </location>
<code_context>
question = self.create_question(
- f"Version [<comment>{version}</comment>]: ", default=version
+ f"Version [<comment>{version}</comment>]: ",
+ default=version if not self.option("disable-package-mode") else "",
)
version = self.ask(question)
</code_context>
<issue_to_address>
Setting version default to empty string may cause confusion in interactive mode.
Consider updating the prompt to clarify that versioning is skipped when package mode is disabled, to avoid user confusion.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
if is_interactive:
question = self.create_question(
f"Version [<comment>{version}</comment>]: ",
default=version if not self.option("disable-package-mode") else "",
)
version = self.ask(question)
=======
if is_interactive:
if self.option("disable-package-mode"):
question = self.create_question(
"Version (skipped, package mode disabled): ",
default="",
)
else:
question = self.create_question(
f"Version [<comment>{version}</comment>]: ",
default=version,
)
version = self.ask(question)
>>>>>>> REPLACE
</suggested_fix>
### Comment 2
<location> `tests/console/commands/test_init.py:1238` </location>
<code_context>
+ assert expected in (source_dir / "pyproject.toml").read_text(encoding="utf-8")
+
+
+def test_init_with_no_package_mode_flag_active_short_flag(
+ tester: CommandTester, source_dir: Path
+) -> None:
+ """Test that poetry init add package-mode = false to pyproject.toml"""
+ inputs = [
+ "my-package", # Package name
+ "", # Version
+ "", # Description
+ "n", # Author
+ "", # License
+ "", # Python
+ "n", # Interactive packages
+ "n", # Interactive dev packages
+ "\n", # Generate
+ ]
+
+ expected = """\
+[tool.poetry]
+package-mode = false
+"""
+ tester.execute("-N", inputs="\n".join(inputs))
+
+ assert (source_dir / "pyproject.toml").exists()
+ assert expected in (source_dir / "pyproject.toml").read_text(encoding="utf-8")
+
+
</code_context>
<issue_to_address>
Consider adding a test for interactive vs non-interactive mode with the new flag.
Please add a test for non-interactive mode to verify that the --disable-package-mode flag works correctly when all options are provided via CLI flags and no prompts are shown.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
def test_init_with_no_package_mode_flag_active(
tester: CommandTester, source_dir: Path
) -> None:
=======
def test_init_with_no_package_mode_flag_active_non_interactive(
tester: CommandTester, source_dir: Path
) -> None:
"""Test that poetry init with --disable-package-mode in non-interactive mode sets package-mode = false."""
# Provide all required options via CLI flags to avoid prompts
package_name = "my-noninteractive-package"
version = "1.2.3"
description = "A non-interactive test package"
author = "Test Author <[email protected]>"
license_ = "MIT"
python = "^3.8"
# Compose the command with all options and the --disable-package-mode flag
command = [
"--name", package_name,
"--version", version,
"--description", description,
"--author", author,
"--license", license_,
"--python", python,
"--disable-package-mode",
"--no-interaction",
]
tester.execute(" ".join(command))
expected = (
"[tool.poetry]\n"
"name = \"my-noninteractive-package\"\n"
"version = \"1.2.3\"\n"
"description = \"A non-interactive test package\"\n"
"authors = [\"Test Author <[email protected]>\"]\n"
"license = \"MIT\"\n"
"python = \"^3.8\"\n"
"package-mode = false\n"
)
pyproject_content = (source_dir / "pyproject.toml").read_text(encoding="utf-8")
assert (source_dir / "pyproject.toml").exists()
assert "package-mode = false" in pyproject_content
assert f'name = "{package_name}"' in pyproject_content
assert f'version = "{version}"' in pyproject_content
assert f'description = "{description}"' in pyproject_content
assert f'authors = ["{author}"]' in pyproject_content
assert f'license = "{license_}"' in pyproject_content
assert f'python = "{python}"' in pyproject_content
def test_init_with_no_package_mode_flag_active(
tester: CommandTester, source_dir: Path
) -> None:
>>>>>>> REPLACE
</suggested_fix>
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
if is_interactive: | ||
question = self.create_question( | ||
f"Version [<comment>{version}</comment>]: ", default=version | ||
f"Version [<comment>{version}</comment>]: ", | ||
default=version if not self.option("disable-package-mode") else "", | ||
) | ||
version = self.ask(question) |
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.
suggestion: Setting version default to empty string may cause confusion in interactive mode.
Consider updating the prompt to clarify that versioning is skipped when package mode is disabled, to avoid user confusion.
if is_interactive: | |
question = self.create_question( | |
f"Version [<comment>{version}</comment>]: ", default=version | |
f"Version [<comment>{version}</comment>]: ", | |
default=version if not self.option("disable-package-mode") else "", | |
) | |
version = self.ask(question) | |
if is_interactive: | |
if self.option("disable-package-mode"): | |
question = self.create_question( | |
"Version (skipped, package mode disabled): ", | |
default="", | |
) | |
else: | |
question = self.create_question( | |
f"Version [<comment>{version}</comment>]: ", | |
default=version, | |
) | |
version = self.ask(question) |
|
||
def test_init_with_no_package_mode_flag_active( | ||
tester: CommandTester, source_dir: Path | ||
) -> None: |
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.
suggestion (testing): Consider adding a test for interactive vs non-interactive mode with the new flag.
Please add a test for non-interactive mode to verify that the --disable-package-mode flag works correctly when all options are provided via CLI flags and no prompts are shown.
def test_init_with_no_package_mode_flag_active( | |
tester: CommandTester, source_dir: Path | |
) -> None: | |
def test_init_with_no_package_mode_flag_active_non_interactive( | |
tester: CommandTester, source_dir: Path | |
) -> None: | |
"""Test that poetry init with --disable-package-mode in non-interactive mode sets package-mode = false.""" | |
# Provide all required options via CLI flags to avoid prompts | |
package_name = "my-noninteractive-package" | |
version = "1.2.3" | |
description = "A non-interactive test package" | |
author = "Test Author <[email protected]>" | |
license_ = "MIT" | |
python = "^3.8" | |
# Compose the command with all options and the --disable-package-mode flag | |
command = [ | |
"--name", package_name, | |
"--version", version, | |
"--description", description, | |
"--author", author, | |
"--license", license_, | |
"--python", python, | |
"--disable-package-mode", | |
"--no-interaction", | |
] | |
tester.execute(" ".join(command)) | |
expected = ( | |
"[tool.poetry]\n" | |
"name = \"my-noninteractive-package\"\n" | |
"version = \"1.2.3\"\n" | |
"description = \"A non-interactive test package\"\n" | |
"authors = [\"Test Author <[email protected]>\"]\n" | |
"license = \"MIT\"\n" | |
"python = \"^3.8\"\n" | |
"package-mode = false\n" | |
) | |
pyproject_content = (source_dir / "pyproject.toml").read_text(encoding="utf-8") | |
assert (source_dir / "pyproject.toml").exists() | |
assert "package-mode = false" in pyproject_content | |
assert f'name = "{package_name}"' in pyproject_content | |
assert f'version = "{version}"' in pyproject_content | |
assert f'description = "{description}"' in pyproject_content | |
assert f'authors = ["{author}"]' in pyproject_content | |
assert f'license = "{license_}"' in pyproject_content | |
assert f'python = "{python}"' in pyproject_content | |
def test_init_with_no_package_mode_flag_active( | |
tester: CommandTester, source_dir: Path | |
) -> None: |
@@ -141,9 +142,12 @@ def _init_pyproject( | |||
|
|||
if is_interactive: | |||
question = self.create_question( | |||
f"Version [<comment>{version}</comment>]: ", default=version | |||
f"Version [<comment>{version}</comment>]: ", | |||
default=version if not self.option("disable-package-mode") else "", |
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.
suggestion (code-quality): Swap if/else branches of if expression to remove negation (swap-if-expression
)
default=version if not self.option("disable-package-mode") else "", | |
default="" if self.option("disable-package-mode") else version, |
Explanation
Negated conditions are more difficult to read than positive ones, so it is bestto avoid them where we can. By swapping the
if
and else
conditions around wecan invert the condition and make it positive.
) | ||
version = self.ask(question) | ||
else: | ||
version = version if not self.option("disable-package-mode") else "" |
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.
suggestion (code-quality): Swap if/else branches of if expression to remove negation (swap-if-expression
)
version = version if not self.option("disable-package-mode") else "" | |
version = "" if self.option("disable-package-mode") else version |
Explanation
Negated conditions are more difficult to read than positive ones, so it is bestto avoid them where we can. By swapping the
if
and else
conditions around wecan invert the condition and make it positive.
@@ -58,6 +58,7 @@ class InitCommand(Command): | |||
flag=False, | |||
multiple=True, | |||
), | |||
option("disable-package-mode", "-N", "Disables package mode.", flag=True), |
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.
I am wondering about the short name. Does it stand for "non-package-mode"? If that is the case, should we name the option non-package-mode
?
@@ -141,9 +142,12 @@ def _init_pyproject( | |||
|
|||
if is_interactive: | |||
question = self.create_question( | |||
f"Version [<comment>{version}</comment>]: ", default=version | |||
f"Version [<comment>{version}</comment>]: ", |
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.
If package-mode is disabled, this now shows a version, which is not the default. We should probably set the version some lines above depending on the disable-package-mode
option.
assert "authors =" not in toml_content | ||
|
||
|
||
def test_init_with_no_package_mode_flag_active_remove_optional_fields_with_email( |
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 should test all fields that may be removed, not just authors
but also version
and description
.
Pull Request Check List
Resolves: #9447
This small pull request adds an option to disable package mode:
-N
and----disable-package-mode
.When this options are enabled optional field in the
pyproject.toml
are skipped if the user leaves the default valueSummary by Sourcery
Add a disable-package-mode option to poetry init that marks package-mode as false and skips default optional metadata fields in the generated pyproject.toml
New Features:
Enhancements:
Tests: