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

export: replace in-tree implementation with poetry-plugin-export #5413

Merged
merged 8 commits into from
Apr 16, 2022

Conversation

abn
Copy link
Member

@abn abn commented Apr 5, 2022

Once the export plugin code releases 1.x (python-poetry/poetry-plugin-export#46), this PR should add changes to replace the export command.

Closes: #4824
Depends-on: #5412

Notes:

  • Maintain commits when merging.
  • Reviewing per commit might be easier.

@abn abn changed the title Change tracking for adopting new export plugin instead of internal export command export: replace in-tree implementation with poetry-plugin-export Apr 5, 2022
@abn abn requested a review from a team April 5, 2022 17:46
@abn abn marked this pull request as ready for review April 5, 2022 17:50
@abn abn added area/cli Related to the command line area/plugin-api Related to plugins/plugin API labels Apr 5, 2022
@abn abn added this to the 1.2 milestone Apr 5, 2022
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.

Before merging this PR, there are some issues in poetry-plugin-export that should be fixed. (Its tests ran against the old in-tree implementation!)

See python-poetry/poetry-plugin-export#50 for proposed changes.

However, the poetry-export-plugin PR is blocked by #5435 which should be merged into master first. Then, this branch can be rebased and python-poetry/poetry-plugin-export#50 can be updated to pass the tests.

Further, there is a change in behavior that I am not sure is intentional: The in-tree export command did not export dev dependencies by default. There was even a test for that default behavior. The poetry-plugin-export exports all non-optional dependencies (including dev-depdendencies) by default.

@abn
Copy link
Member Author

abn commented Apr 10, 2022

Thanks @radoering. Ugg; good catches. As for the beaviour change, I commented on the python-poetry/poetry-plugin-export#50. I need to take a closer look at #5435, first glance reminds of #5311 (comment).

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.

I think there two open points that build on each other:

  1. decision on default behavior (including --with/--without handling), see latest changes in Fix 1.0.0 issues poetry-plugin-export#50 for the variant "no breaking change but different behavior of export command compared to install command"
  2. update documentation (should be done after decision on default behavior was made) btw, the documentation has to remain in this repo?

@abn
Copy link
Member Author

abn commented Apr 11, 2022

poetry-plugin-export==1.0.1 should fix the issues. I have also added an additional test run to verify everything works.

@abn abn requested a review from radoering April 11, 2022 18:56
@radoering
Copy link
Member

2. update documentation (should be done after decision on default behavior was made) btw, the documentation has to remain in this repo?

Still needs to be done.

@abn
Copy link
Member Author

abn commented Apr 12, 2022

Ack, will update.

Until we have a better strategy for handling this, documentation should remain in this repo as the current website publishing only accounts for this repository. But, in the mid-term (outside of this PR), figure out how to get the website to also support any documentation we put in the plugin repository.

The export plugin is a snow flake at the moment, so handling it as is for now is okay I reckon. But if we start adding more "bundled" plugins, we should come up with a better solution sooner.

@radoering radoering merged commit dca0c56 into python-poetry:master Apr 16, 2022
@abn abn mentioned this pull request Jun 6, 2022
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/cli Related to the command line area/plugin-api Related to plugins/plugin API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants