Skip to content

fix: recursively include packages for conda using setuptools#649

Merged
mxschmitt merged 1 commit intomicrosoft:masterfrom
kumaraditya303:master
Apr 27, 2021
Merged

fix: recursively include packages for conda using setuptools#649
mxschmitt merged 1 commit intomicrosoft:masterfrom
kumaraditya303:master

Conversation

@kumaraditya303
Copy link
Copy Markdown
Contributor

@kumaraditya303 kumaraditya303 commented Apr 25, 2021

No description provided.

mxschmitt
mxschmitt reviewed Apr 25, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@mxschmitt mxschmitt left a comment

Choose a reason for hiding this comment

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

Either you use

find_packages(exclude=["tests*", "playwright._*"])

which will result in

['playwright', 'playwright.async_api', 'playwright.sync_api']

or you just hard-code the 3 output values of it. Having it like you suggested would result in

['playwright', 'tests.common', 'tests.async', 'tests.sync', 'playwright.async_api', 'playwright.sync_api', 'playwright._impl']

which is not expected from my understanding.

@kumaraditya303
Copy link
Copy Markdown
Contributor Author

kumaraditya303 commented Apr 25, 2021 •
edited
Loading

Did you mean include instead of exclude ? We do need playwright._impl

@mxschmitt
Copy link
Copy Markdown
Contributor

mxschmitt commented Apr 25, 2021

Did you mean include instead of exclude ?

Just having packages=['playwright', 'playwright.async_api', 'playwright.sync_api'] as before and adding sync/async packages to the list which should result in the same and has less magic.

@kumaraditya303
Copy link
Copy Markdown
Contributor Author

kumaraditya303 commented Apr 25, 2021 •
edited
Loading

packages=['playwright', 'playwright.async_api', 'playwright.sync_api'] do you want to hard code each value, in that case we do need _impl package

@mxschmitt
Copy link
Copy Markdown
Contributor

mxschmitt commented Apr 25, 2021

packages=['playwright', 'playwright.async_api', 'playwright.sync_api'] do you want to hard code each value, in that case we do need _impl package

ah I guessed impl was not expected there. 👍 Then feel free to adjust it to find_packages(exclude=["tests*"])

@mxschmitt mxschmitt requested a review from pavelfeldman April 25, 2021 12:43
@mxschmitt mxschmitt merged commit 890a833 into microsoft:master Apr 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pavelfeldman pavelfeldman Awaiting requested review from pavelfeldman

1 more reviewer

@mxschmitt mxschmitt mxschmitt approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@kumaraditya303 @mxschmitt