feat: add support for project-specific min-release-age configuration#10898
feat: add support for project-specific min-release-age configuration#10898finswimmer wants to merge 1 commit into
Conversation
b96e260 to
c265539
Compare
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
_load_project_config, unknown keys intool.poetry.configare currently silently ignored; consider explicitly validating and failing on unknown keys so that misconfigurations are surfaced early and users get a clear error message. - The configuration-precedence explanation for
[tool.poetry.config](overriding project-local and global config but still overridden by env vars) is repeated in multiple places in the docs; consider consolidating this into a single referenced section to reduce duplication and prevent the descriptions from drifting out of sync.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `_load_project_config`, unknown keys in `tool.poetry.config` are currently silently ignored; consider explicitly validating and failing on unknown keys so that misconfigurations are surfaced early and users get a clear error message.
- The configuration-precedence explanation for `[tool.poetry.config]` (overriding project-local and global config but still overridden by env vars) is repeated in multiple places in the docs; consider consolidating this into a single referenced section to reduce duplication and prevent the descriptions from drifting out of sync.
## Individual Comments
### Comment 1
<location path="tests/test_factory.py" line_range="635-644" />
<code_context>
+ Factory().create_poetry(project_dir)
+
+
+def test_project_config_overrides_poetry_toml(tmp_path: Path) -> None:
+ project_dir = _make_project(
+ tmp_path,
+ {"solver": {"min-release-age": 14}},
+ poetry_toml={"solver": {"min-release-age": 7}},
+ )
+ poetry = Factory().create_poetry(project_dir)
+ assert poetry._config.get("solver.min-release-age") == 14
+
+
</code_context>
<issue_to_address>
**suggestion (testing):** Consider extending override tests to cover array-based options as well, not just `min-release-age`.
To better show precedence and merging, add a similar test for at least one list-based option (e.g. `min-release-age-exclude` or `min-release-age-exclude-source`) where `poetry.toml` and `pyproject.toml` define different lists and the project config is asserted to win.
```suggestion
def test_project_config_overrides_poetry_toml(tmp_path: Path) -> None:
project_dir = _make_project(
tmp_path,
{"solver": {"min-release-age": 14}},
poetry_toml={"solver": {"min-release-age": 7}},
)
poetry = Factory().create_poetry(project_dir)
assert poetry._config.get("solver.min-release-age") == 14
def test_project_config_overrides_poetry_toml_array_option(tmp_path: Path) -> None:
project_dir = _make_project(
tmp_path,
{"solver": {"min-release-age-exclude": ["foo", "bar"]}},
poetry_toml={"solver": {"min-release-age-exclude": ["baz"]}},
)
poetry = Factory().create_poetry(project_dir)
assert poetry._config.get("solver.min-release-age-exclude") == ["foo", "bar"]
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
Deploy preview for website ready!
Deployed with vercel-action |
c265539 to
d65ff00
Compare
I think the main question is whether we assume that this is just a special case or whether we expect more config settings to be allowed in pyproject.toml in the future. In the first case, it probably does not matter much where we put it. In the latter case, it probably makes sense to have the |
That are my thoughts as well. I tend to say this is a special case. But at the end of the day I have no strong opinion here. |
Pull Request Check List
Poetry 2.4 introduces options to cool down dependency resolutions. These options are only available as configuration options for the Poetry installation itself.
If working on multiple projects especially in a corporate environment it might be necessary to have these options available on a per-project base. This is also useful if renovate or dependabot are used to update the dependencies.
For now i put these option under
[tool.poetry.config]. But maybe we want to put them directly under[tool.poetry](withoutsolver.prefix)?