Don't change poetry.lock during poetry update --dry-run#3767
Don't change poetry.lock during poetry update --dry-run#3767martinmo wants to merge 1 commit intopython-poetry:masterfrom martinmo:3766-update-dry-run
poetry update --dry-run#3767Conversation
|
Linting fails on a file that is not touched in this PR (docs/docs/pyproject.md). |
|
@martinmo can you rebase the branch please? Should fix the linting issues. |
Done ✅ |
|
I have rebased this to check if it works against the current code in |
|
Hello @martinmo, could you please rebase once more to force the tests running again? Thanks! fin swimmer |
Done ✅ |
|
Hi @martinmo just checking on this and seeing if you had the time to push this through; it's a pretty annoying part of poetry! Thanks :) |
Not yet, but I rebased and force pushed it again to see what the CI checks say. |
|
Hi @abn could you please have another look at this PR? I rebased it and adjusted it to conform to the new coding guidelines. Thank you! |
|
@finswimmer the review for this PR is somehow stuck. I saw that you have been active on other PRs in the last days, could you please have a look at this PR? It has a test for the changed behavior and all CI checks have passed. Thanks! |
|
Me + team affected from this, would be great if it could be progressed. |
|
I'm on 1.2.0b1 (current latest) and this issue persist. Please proceed with the PR. Much appreciated. |
radoering
left a comment
There was a problem hiding this comment.
At first, thanks for your contribution and your patience.
I think the fix may not cover all cases. I'd really appreciate it if you could write tests for the other commands (see below) and check if my assumption is correct.
|
|
||
| def _write_lock_file(self, repo: Repository, force: bool = True) -> None: | ||
| def _write_lock_file(self, repo: Repository, force: bool = False) -> None: | ||
| if force or (self._update and self._write_lock): |
There was a problem hiding this comment.
| if force or (self._update and self._write_lock): | |
| if (force or self._update) and self._write_lock: |
I think that line is also wrong since self._write_lock is only used for --dry-run, force should not overwrite this. From inspecting the code, I assume that fix is necessary for poetry add --lock --dry-run. I really think we need more coverage, here.
| ) | ||
|
|
||
|
|
||
| def test_dry_run_update( |
There was a problem hiding this comment.
Can you please add similar tests (or extend existing tests) for the following commands (if not yet existing):
add --dry-runadd --lock --dry-runremove --dry-run
Further, it seems that tests for add check that the pyproject.toml is not changed for dry-run. Can you please add this check here, too?
There was a problem hiding this comment.
And probably, you should rebase first before adding tests. 🙏
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Pull Request Check List
Resolves: #3766
AFAIK, there is no relevant documentation to be changed, am I right?