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

Handle nested wildcards in package includes correctly. Fixes: #1379. #1592

Merged
merged 2 commits into from Jan 7, 2020

Conversation

@stephsamson
Copy link
Member

stephsamson commented Nov 17, 2019

Pull Request Check List

  • Added tests for changed code.

This PR fixes #1379. I've also added some unit tests for PackageInclude.

@stephsamson stephsamson added the Bug label Nov 17, 2019
@stephsamson stephsamson added this to the 1.0 milestone Nov 17, 2019
@stephsamson stephsamson requested review from sdispater and finswimmer Nov 17, 2019
Copy link
Member

sdispater left a comment

Thanks for your first contribution! :-)

Overall great job 👍 However, we need tests for this use case in the sdist builders because I think they will fail since we check for the presence of an __init__.py file in one other place: https://github.com/sdispater/poetry/blob/master/poetry/masonry/builders/sdist.py#L237

root = self._elements[0]
if root.name != "__init__.py":

This comment has been minimized.

Copy link
@sdispater

sdispater Nov 18, 2019

Member

I think we can change this line to check if there is at least one Python file, regardless of whether it's a __init__.py file or not. If there is no Python file, it's not a package and we should raise the exception.

This comment has been minimized.

Copy link
@stephsamson

stephsamson Jan 6, 2020

Author Member

Thanks, I've also added it now in commit 1d0e99d.

stephsamson added 2 commits Nov 17, 2019
…not necessarily an __init__.py file) will be included.
@stephsamson stephsamson force-pushed the bug/wildcards branch from d789130 to 1d0e99d Jan 6, 2020
@stephsamson stephsamson requested a review from sdispater Jan 6, 2020
Copy link
Member

sdispater left a comment

Looks good to me 👍 Thanks!

@sdispater sdispater merged commit c401850 into master Jan 7, 2020
32 checks passed
32 checks passed
Linting
Details
Linting
Details
Linux (2.7)
Details
Linux (2.7)
Details
Linux (3.5)
Details
Linux (3.5)
Details
Linux (3.6)
Details
Linux (3.6)
Details
Linux (3.7)
Details
Linux (3.7)
Details
Linux (3.8)
Details
Linux (3.8)
Details
MacOS (2.7)
Details
MacOS (2.7)
Details
MacOS (3.5)
Details
MacOS (3.5)
Details
MacOS (3.6)
Details
MacOS (3.6)
Details
MacOS (3.7)
Details
MacOS (3.7)
Details
MacOS (3.8)
Details
MacOS (3.8)
Details
Windows (2.7)
Details
Windows (2.7)
Details
Windows (3.5)
Details
Windows (3.5)
Details
Windows (3.6)
Details
Windows (3.6)
Details
Windows (3.7)
Details
Windows (3.7)
Details
Windows (3.8)
Details
Windows (3.8)
Details
@sdispater sdispater added this to Closed in Bugs via automation Jan 7, 2020
@sdispater sdispater deleted the bug/wildcards branch Jan 7, 2020
@StephenBrown2 StephenBrown2 mentioned this pull request Jan 10, 2020
5 of 5 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Bugs
  
Closed
2 participants
You can’t perform that action at this time.