Skip to content
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

reword the --with help text and deprecate --without #212

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

jonapich
Copy link

This fixes #299

  • The help text for --with has been updated to express that groups are never included by default
  • The readme has been updated with this precision as well
  • The --without option has been marked deprecated since it serves no purpose anymore.

Deprecation rationale:

  • No groups are ever included by default, so --without cannot remove anything
  • --only already disables --with and --without (ref)
  • Therefore, --without can only remove groups that the user also provided in --with, which means that the priority (who wins? with or without?) is an implementation detail that isn't documented.

@sonarcloud
Copy link

sonarcloud bot commented Jun 16, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Copy link
Member

@radoering radoering left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--without main is still a thing, but since --with group --without main is the same as --only group here, there is probably no need for --without.

README.md Outdated
* `--without`: The dependency groups to ignore when exporting.
* `--with`: The optional dependency groups to include when exporting.
* `--only`: The only dependency groups to include when exporting.
* `--with`: The dependency groups to include. By default, no groups are exported.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main group is exported by default.

@jonapich
Copy link
Author

pre-commit.ci autofix

@sonarcloud
Copy link

sonarcloud bot commented Jul 19, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Comment on lines +72 to 74
* {{< option name="without" deprecated=true >}}The dependency groups to ignore.{{< /option >}}
* {{< option name="default" deprecated=true >}}Only export the main dependencies.{{< /option >}}
* {{< option name="dev" deprecated=true >}}Include development dependencies.{{< /option >}}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if/where this syntax works. In the end, it's https://github.com/python-poetry/poetry/blob/88096aca2300fee8d1d0babdb505e1cf50cbea17/docs/cli.md?plain=1#L706-L737 what will be displayed on the website anyway. Probably, we should choose the simpler syntax (without {{< option).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this page published at all? I found it weird to have to edit the same information at 2 places. I couldn't find the first sentence of this file through a google search:

The export plugin allows the export of locked packages to various formats.

Should these files simply be deleted?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Afaik, it's not published. However, I suppose it was the target to integrate it somehow in the poetry docs and remove the the command from https://github.com/python-poetry/poetry/blob/master/docs/cli.md. Thus, deleting it would be a step backwards. On the other hand, python-poetry/poetry#5980 got stuck since python-poetry/poetry#5980 (comment).

I just noticed, there is even a third place: https://github.com/python-poetry/poetry-plugin-export/blob/main/README.md

Since I'm not much into this topic, I'd probably just update all three spots. 🤷

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

poetry export doesn't include non-optional groups
2 participants