new command to support more options
Adding all the missing options that are asked interactively by the init command.
Also improving & fixing the doc of init command to match.
Question: How to validate the rendering of docs/cli.md (especially the relref)?
-> Answered in https://github.com/python-poetry/poetry/pull/5708#discussion_r885083371
Question: Is it on purpose that mypy is not part of the pre-commit hooks? My 1st push failed on CI because of a mypy error because I didn't realize I should've ran it manually.
-> Answered in https://github.com/python-poetry/poetry/pull/5710#issuecomment-1146531780
Pull Request Check List
Resolves: #1854
- [x] Added tests for changed code.
- [x] Updated documentation for changed code.
Deploy preview for website ready!
✅ Preview https://website-67l9ix9lp-python-poetry.vercel.app
Built with commit 16caa173ba761b487fab50d44bba57b5c7fc879f. This pull request is being automatically deployed with vercel-action
Question: Is it on purpose that
mypyis not part of the pre-commit hooks? My 1st push failed on CI because of a mypy error because I didn't realize I should've ran it manually.
Yes, it is for now. mypy was part of our pre-commit hooks but we can't run it in the correct environment without duplicating our list of dependencies. We are probably going to set it up as a tox environment and then suggest running tox to trigger pytest and mypy.
There are some existing commands that inherit the InitCommand. Should those be also detached by extracting shared helpers? Maybe not yet in this PR though.
Also, let's drop the -l short option for --license as it's inconsistent and it was never documented.
Added in commit https://github.com/python-poetry/poetry/pull/5710/commits/7d578a10d3d0f1074f94a48fe3ea90f67ddebfa1.
I would prefer to see the common parts of both commands factored out into shared helpers.
Done in commit https://github.com/python-poetry/poetry/pull/5710/commits/a09c47b38a15c2803c8bec159f857258eac80838.
@neersighted is this ok? or something to improve still?
@neersighted I tried to address the review comments but looks like this PR was forgotten and there are some conflicts to be resolved. Those should be easy to fix, but would it be possible to have a review for the commits that I added since you last checked this?
Hi @juhoautio -- the maintainers are busy people doing this in their spare time. While an occasional ping (especially to those who have already interacted with a PR/issue) is not unwelcome, pinging people who have not expressed interest in an issue or reviewing code, without a prior understanding of what they are interested in helping with, is generally considered rude. Likewise, pinging repeatedly is generally considered rude.
Additionally, Github as a 'request review/re-review' feature that is generally used for this and makes it easier to reason about what reviews are left on your to-do list as a contributor with many priorities.
Most reviewers will not take a look at code that is conflicted and failing tests as non-mergable code must, by its nature, change (often substantially) in order to be accepted. Thus reviewing it is generally a waste of time for all parties involved. I am sorry that I did not comment that here -- hopefully that explanation/expectation helps in the future when you request reviews.
Sorry! I had not noticed that there's a Re-request review feature, or at least didn't remember. I'll use that from now on.
I'm also very sorry that I ended up using desperate means like pinging another maintainer. In fact I wasn't sure how to best raise my question about what I should be doing next without appearing rude, yet I failed at it. In the end, I now have the guidance that I was looking for. Thanks a lot for that.
The test failure with Tests / macOS / 3.11-dev (pull_request) doesn't seem to be related to my changes. The same failure seems to be happening at least in one other open PR.
Should the existing --dev-dependency option of init be deprecated?
How about having a new option --group-dependency for both init and new? The value format could be GROUP:DEPENDENCY.
e.g. dev:requests:^2.10.0 or dev:requests=2.11.1 or dev:requests.
or just drop it in general as it's going to be pretty niche during new
The ability to set dev dependencies with new is needed for my use case. Or then I would like to have an option for add that allows only modifying pyproject.toml without updating poetry.lock.
I think the best move is to drop the --dev-dependency change from this PR, and then introduce a new PR deprecating --dev-dependency on init and adding --group-dependency using the design you've proposed.
there is a lot of moving code around in this PR in the same commit it was changed, so it's a bit hard to review.
This is unfortunate, yes. The commit to blame for most of the refactoring is https://github.com/python-poetry/poetry/pull/5710/commits/a09c47b38a15c2803c8bec159f857258eac80838. In that commit I am only moving the existing code to functions but not changing it otherwise.
I think we should block this on #6669 as the new
AbstractRepositoryintroduced in that PR will make all the functions that currently takePool(which just hasPyPiRepositoryin it anyway) much cleaner and allow for droppingPooluntil such time that we actually support searching sources other than PyPI.
Looks like that PR was already merged 🎉 I'm sorry but I couldn't figure out what it would mean in practice for this PR, how to replace Pool with AbstractRepository in these functions? I was about to take AbstractRepository in find_best_version_for_package but it ends up using VersionSelector(pool). Is it possible to change the signature of VersionSelector to take AbstractRepository instead of Pool?
I committed something along these lines, curious to hear the feedback (commit https://github.com/python-poetry/poetry/pull/5710/commits/b3e5364a3dbc0748be568372bad362191786c864). But I rolled it back because it seemed to be trying real PyPI calls and made the build fail on Travis. I would also like to omit it to keep this PR simpler.
@juhoautio: Sorry I'm late to the party. I think we should rework the implementation of poetry new and poetry init in that way, that poetry new becomes an extended version of poetry init. Doing this we can makes sure, that new have at least the same options as init, even if we introduce a new option.
This is why I closed the issue you've linked in your PR description. The new "major" ticket should be #2563 now.
@finswimmer is someone (you?) going to work on implementing #2563 some time soon? If yes, does that means this PR is rejected after all? But if no one will work on #2563, I would still hope to have this PR merged with the current scope ie. just adding some missing options & fixing some docs without any bigger refactoring.
Hey @juhoautio ,
this doesn't mean your PR is rejected. The scope is not that far away from #2563 and I was hoping that you maybe like to go that extra mile. :)
fin swimmer
I could work on it if you'd like, but if the design changes that much, I think starting from a clean slate with a new PR could be better. I would hope to have some guidance on the desired approach for implementing it. Please let me know.
I'm still wondering if this PR could be finished because it's almost there. Maybe I'm also missing what additional use cases the new alternative will enable compared to this.
Superseded by #9088
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.