Navigation Menu

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

(ISSUE #146) Adds license and long_description for pypi #154

Merged
merged 2 commits into from May 9, 2012

Conversation

silent1mezzo
Copy link
Contributor

This adds both the license and long_description to pypi.

It just uses the contents from LICENSE and README.markdown to populate these fields.

@silent1mezzo
Copy link
Contributor Author

One other thing. I tried automatically adding the version from reversion/__init__.py but because you're importing django as well there's no simple way to do it.

Couple of solutions I thought of:

  1. Read __init__.py and then parse through with a regex searching for the VERSION
  2. Create reversion/version.py and import that into __init__.py and setup.py

The second solution is much cleaner IMO but it requires an extra file and it's up to you. This would mean you'd only need to update the version in that one place and you could have it import into setup.py for both version and for your download links.

@charettes
Copy link
Contributor

First solution is definitely not good idea.

@silent1mezzo
Copy link
Contributor Author

I agree with @charettes, I just included it because some other large projects use that.

@etianen
Copy link
Owner

etianen commented May 9, 2012

Your code will still be embedding markdown syntax in pypi. I suppose that's better than the broken ascii!

What's the problem with

from reversion import VERSION

In any case, solution 2 is clearly better!

@etianen etianen mentioned this pull request May 9, 2012
@silent1mezzo
Copy link
Contributor Author

from reversion import VERSION dies in fire because __init__.py also includes import django which exceptions out when pypi runs setup.py

@etianen
Copy link
Owner

etianen commented May 9, 2012

Ah, I see. D'oh!

If you can implement your solution 2, then I'll gladly pull this in.

@silent1mezzo
Copy link
Contributor Author

A little more hacky than I usually like to make sure everything is backwards compatible.

@@ -0,0 +1 @@
__version__ = (1, 6, 0)
Copy link
Owner

Choose a reason for hiding this comment

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

Why the version here? Isn't this just adding lines of code for no reason?

Copy link
Owner

Choose a reason for hiding this comment

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

Nice markdown formatting error there. I mean __version__

Copy link
Contributor Author

Choose a reason for hiding this comment

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

__version__ is the PEP standard. Because you've used VERSION every where else I didn't want to break backwards compatibility incase anyone is relying on VERSION. http://www.python.org/dev/peps/pep-0396/

etianen added a commit that referenced this pull request May 9, 2012
(ISSUE #146) Adds license and long_description for pypi
@etianen etianen merged commit c1e3d49 into etianen:master May 9, 2012
@etianen
Copy link
Owner

etianen commented May 9, 2012

You're pulled! Many thanks! I've learned something today. :)

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.

None yet

4 participants