feat(version): add development release bump option#10942
Open
Yijian6 wants to merge 1 commit into
Open
Conversation
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The
bump_rulesset is recreated on everyincrement_versioncall; consider moving it to a class-level or module-level constant to avoid repeated allocation and to centralize the definition of supported bump rules. - When
--devis used with a non-bump rule (e.g. an explicit version string), it is silently ignored; consider either raising a clear error or logging a message so users know--devonly applies to the defined bump rules.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `bump_rules` set is recreated on every `increment_version` call; consider moving it to a class-level or module-level constant to avoid repeated allocation and to centralize the definition of supported bump rules.
- When `--dev` is used with a non-bump rule (e.g. an explicit version string), it is silently ignored; consider either raising a clear error or logging a message so users know `--dev` only applies to the defined bump rules.
## Individual Comments
### Comment 1
<location path="tests/console/commands/test_version.py" line_range="92-107" />
<code_context>
assert command.increment_version(version, rule, True).text == expected
+@pytest.mark.parametrize(
+ "version, rule, next_phase, expected",
+ [
+ ("1.1.0", "major", False, "2.0.0.dev0"),
+ ("2.0.0.dev0", "major", False, "2.0.0.dev1"),
+ ("1.1.0", "minor", False, "1.2.0.dev0"),
+ ("1.1.0", "patch", False, "1.1.1.dev0"),
+ ("1.1.0", "premajor", False, "2.0.0a0.dev0"),
+ ("1.1.0", "preminor", False, "1.2.0a0.dev0"),
+ ("1.1.0", "prepatch", False, "1.1.1a0.dev0"),
+ ("1.1.0", "prerelease", False, "1.1.1a0.dev0"),
+ ("1.1.1a0.dev0", "prerelease", False, "1.1.1a0.dev1"),
+ ("1.1.0a2.dev1", "prerelease", True, "1.1.0b0.dev0"),
+ ],
+)
+def test_dev_version(
+ version: str,
+ rule: str,
+ next_phase: bool,
+ expected: str,
+ command: VersionCommand,
+) -> None:
+ assert (
+ command.increment_version(version, rule, next_phase, dev=True).text == expected
+ )
</code_context>
<issue_to_address>
**suggestion (testing):** Add edge-case test where `--dev` bumps from a dev version to a different base (e.g. `1.1.0.dev0` → `1.2.0.dev0` with `minor --dev`).
Current parameters cover creating dev releases from stable versions and incrementing dev within the same base (e.g. `2.0.0.dev0` + major → `2.0.0.dev1`). Please also add a case where the starting version is already a dev release but the bump changes the base, e.g.:
```python
("1.1.0.dev0", "minor", False, "1.2.0.dev0"),
```
in `test_dev_version`. This will assert that `--dev` resets to `.dev0` when the base version changes, matching the `new == parsed.without_devrelease()` condition.
```suggestion
@pytest.mark.parametrize(
"version, rule, next_phase, expected",
[
("1.1.0", "major", False, "2.0.0.dev0"),
("2.0.0.dev0", "major", False, "2.0.0.dev1"),
("1.1.0", "minor", False, "1.2.0.dev0"),
("1.1.0.dev0", "minor", False, "1.2.0.dev0"),
("1.1.0", "patch", False, "1.1.1.dev0"),
("1.1.0", "premajor", False, "2.0.0a0.dev0"),
("1.1.0", "preminor", False, "1.2.0a0.dev0"),
("1.1.0", "prepatch", False, "1.1.1a0.dev0"),
("1.1.0", "prerelease", False, "1.1.1a0.dev0"),
("1.1.1a0.dev0", "prerelease", False, "1.1.1a0.dev1"),
("1.1.0a2.dev1", "prerelease", True, "1.1.0b0.dev0"),
],
)
def test_dev_version(
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+92
to
+107
| @pytest.mark.parametrize( | ||
| "version, rule, next_phase, expected", | ||
| [ | ||
| ("1.1.0", "major", False, "2.0.0.dev0"), | ||
| ("2.0.0.dev0", "major", False, "2.0.0.dev1"), | ||
| ("1.1.0", "minor", False, "1.2.0.dev0"), | ||
| ("1.1.0", "patch", False, "1.1.1.dev0"), | ||
| ("1.1.0", "premajor", False, "2.0.0a0.dev0"), | ||
| ("1.1.0", "preminor", False, "1.2.0a0.dev0"), | ||
| ("1.1.0", "prepatch", False, "1.1.1a0.dev0"), | ||
| ("1.1.0", "prerelease", False, "1.1.1a0.dev0"), | ||
| ("1.1.1a0.dev0", "prerelease", False, "1.1.1a0.dev1"), | ||
| ("1.1.0a2.dev1", "prerelease", True, "1.1.0b0.dev0"), | ||
| ], | ||
| ) | ||
| def test_dev_version( |
There was a problem hiding this comment.
suggestion (testing): Add edge-case test where --dev bumps from a dev version to a different base (e.g. 1.1.0.dev0 → 1.2.0.dev0 with minor --dev).
Current parameters cover creating dev releases from stable versions and incrementing dev within the same base (e.g. 2.0.0.dev0 + major → 2.0.0.dev1). Please also add a case where the starting version is already a dev release but the bump changes the base, e.g.:
("1.1.0.dev0", "minor", False, "1.2.0.dev0"),in test_dev_version. This will assert that --dev resets to .dev0 when the base version changes, matching the new == parsed.without_devrelease() condition.
Suggested change
| @pytest.mark.parametrize( | |
| "version, rule, next_phase, expected", | |
| [ | |
| ("1.1.0", "major", False, "2.0.0.dev0"), | |
| ("2.0.0.dev0", "major", False, "2.0.0.dev1"), | |
| ("1.1.0", "minor", False, "1.2.0.dev0"), | |
| ("1.1.0", "patch", False, "1.1.1.dev0"), | |
| ("1.1.0", "premajor", False, "2.0.0a0.dev0"), | |
| ("1.1.0", "preminor", False, "1.2.0a0.dev0"), | |
| ("1.1.0", "prepatch", False, "1.1.1a0.dev0"), | |
| ("1.1.0", "prerelease", False, "1.1.1a0.dev0"), | |
| ("1.1.1a0.dev0", "prerelease", False, "1.1.1a0.dev1"), | |
| ("1.1.0a2.dev1", "prerelease", True, "1.1.0b0.dev0"), | |
| ], | |
| ) | |
| def test_dev_version( | |
| @pytest.mark.parametrize( | |
| "version, rule, next_phase, expected", | |
| [ | |
| ("1.1.0", "major", False, "2.0.0.dev0"), | |
| ("2.0.0.dev0", "major", False, "2.0.0.dev1"), | |
| ("1.1.0", "minor", False, "1.2.0.dev0"), | |
| ("1.1.0.dev0", "minor", False, "1.2.0.dev0"), | |
| ("1.1.0", "patch", False, "1.1.1.dev0"), | |
| ("1.1.0", "premajor", False, "2.0.0a0.dev0"), | |
| ("1.1.0", "preminor", False, "1.2.0a0.dev0"), | |
| ("1.1.0", "prepatch", False, "1.1.1a0.dev0"), | |
| ("1.1.0", "prerelease", False, "1.1.1a0.dev0"), | |
| ("1.1.1a0.dev0", "prerelease", False, "1.1.1a0.dev1"), | |
| ("1.1.0a2.dev1", "prerelease", True, "1.1.0b0.dev0"), | |
| ], | |
| ) | |
| def test_dev_version( |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a
--devoption topoetry versionso existing bump rules can create or advance PEP 440 development releases.Resolves: #8718
Why
The issue discussion settled on using
--devwith the existing bump rules instead of adding an ambiguous standalonedevbump rule. This keeps version bumps moving forward and avoids the previous PR's decrement cases such as moving from a prerelease to an earlier dev release.How
first_devrelease(),next_devrelease(), andwithout_devrelease()helpers.--devonly to known bump rules..devNwhen the selected bump rule already targets the current development release base; otherwise creates.dev0on the bumped target.--devremove the development release suffix for the discussed release cases.Testing
python -m pytest tests/console/commands/test_version.py -qpython -m ruff check src/poetry/console/commands/version.py tests/console/commands/test_version.pypython -m ruff format --check src/poetry/console/commands/version.py tests/console/commands/test_version.pypython -m mypy src/poetry/console/commands/version.py tests/console/commands/test_version.pygit diff --checkBreaking changes
None.
Pull Request Check List