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

Added Manifest file #31

Merged
merged 1 commit into from
Oct 21, 2017
Merged

Added Manifest file #31

merged 1 commit into from
Oct 21, 2017

Conversation

kszucs
Copy link
Contributor

@kszucs kszucs commented Oct 20, 2017

@Tinche Tinche merged commit d366624 into Tinche:master Oct 21, 2017
@Tinche
Copy link
Owner

Tinche commented Oct 21, 2017

Thanks! Do you need me to do a release to PyPI?

@kxepal
Copy link

kxepal commented Oct 21, 2017

The only file that should be mentioned there is LICENSE. Others are redundant and get included by default.

@kszucs
Copy link
Contributor Author

kszucs commented Oct 22, 2017

@Tinche a release would be great! Thank You!

@kszucs
Copy link
Contributor Author

kszucs commented Oct 23, 2017

@Tinche please ping me here conda-forge/staged-recipes#4192 after the release

recursive-include aiofiles *.py

include README.rst
include MANIFEST.in

Choose a reason for hiding this comment

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

Sorry for a comment after merging but MANIFEST.in is not needed in the file.
Maybe it doesn't make a harm but no other project has the line IMHO.
I'm pretty sure aio-libs: aiohttp, yarl, multidict and others works pretty well.

Copy link

Choose a reason for hiding this comment

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

MANIFEST.in, MANIFEST and setup.py don’t need to be in MANIFEST.in, indeed.

@@ -0,0 +1,6 @@
recursive-include aiofiles *.py

Choose a reason for hiding this comment

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

The line should be removed: setuptools/distutils has a rule to include all .py files from mentioned packages.

Copy link

Choose a reason for hiding this comment

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

This is true.

@Tinche
Copy link
Owner

Tinche commented Nov 30, 2017

Uh, it works like it is now, so I'm not in a rush to fix it. If someone wants to submit a cleanup PR, I'll be happy to merge it in :)

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.

5 participants