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

add license title and fix short identifier #38

Merged
merged 2 commits into from
Jul 31, 2016
Merged

add license title and fix short identifier #38

merged 2 commits into from
Jul 31, 2016

Conversation

waldyrious
Copy link
Contributor

@waldyrious waldyrious commented Jul 31, 2016

The license title is not strictly required, but it's useful metadata, and part of the recommended license template text:

The short identifier has been codified by both SPDX and OSI as "ISC" -- see https://opensource.org/licenses/ISC and http://spdx.org/licenses/ISC.html.

@waldyrious waldyrious changed the title add license title add license title and fix short identifier Jul 31, 2016
packages=['bidict'],
package_data=dict(bidict=['VERSION']),
zip_safe=True,
classifiers=[
'Development Status :: 4 - Beta',
'Intended Audience :: Developers',
'License :: OSI Approved :: ISC License (ISCL)',
'License :: OSI Approved :: ISC License',
Copy link
Owner

@jab jab Jul 31, 2016

Choose a reason for hiding this comment

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

https://pypi.python.org/pypi?%3Aaction=list_classifiers lists the classifier as I had it, with (ISCL) at the end, which is where I got this from. Can you provide a reference explaining why that's wrong and this is right?

Copy link
Contributor Author

@waldyrious waldyrious Jul 31, 2016

Choose a reason for hiding this comment

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

Sure. The second commit has the to the relevant authoritative sources in the extended description:

see https://opensource.org/licenses/ISC and http://spdx.org/licenses/ISC.html

Sorry that I didn't make this clearer in the PR's main description text. I'll add it now, for future reference.

By the way, I did submit a PR to fix the acronym in the list of classifiers (https://github.com/russianidiot/classifiers.py/pull/1) [2020-06-20 update: I now submitted it to the proper repo: https://github.com/pypa/trove-classifiers/pull/35]. I'm not entirely sure that is the correct procedure to go about fixing this, but if it isn't, I expect that PR to at least open the conversation to reach that goal.

@jab
Copy link
Owner

jab commented Jul 31, 2016

Obrigado pelo PR @waldyrious! Happy to merge the first change and just have some questions before merging the second. If you could explain the consequences of making your changes to setup.py vs. leaving it as-is, I'm curious to understand better why this makes a difference.

Thanks again for dropping by, and if you'd like to share how you came across / are using bidict, I'm always interested to connect more with the community. (Was it through https://en.wikipedia.org/wiki/Bidirectional_map?)

@waldyrious
Copy link
Contributor Author

Thanks for the quick response! I hope I have clarified your questions in the comments above.

As for how I came across this package, it was actually precisely due to your choice of license :) I happen to think the ISC is the neatest of the short, permissive licenses around (MIT, BSD, etc.), so I decided to make sure it was being properly applied at in the most popular github repos (100+ stars) using it.

@jab jab merged commit 576ef85 into jab:master Jul 31, 2016
@jab
Copy link
Owner

jab commented Jul 31, 2016

Thank you for the thorough explanations! Just merged this. I suppose I could have waited to see what happens with russianidiot/classifiers.py#1 first (thanks for following up there, by the way), but I'm happy to just watch that issue for now and for bidict to lead the charge in using your preferred classifier in the meantime.

And by the way, I think that result I turned up about license values in setup.py was incorrect and outdated; according to https://packaging.python.org/distributing/#license license "should provide the type of license you are using" after all.

@waldyrious
Copy link
Contributor Author

I'm glad you decided to lead the charge! standardization FTW :)

Thanks for the info on what should go on the license field. I did think it was odd that they'd recommend the license file. I opened up a PR proposing a fix, but given the age of the other open PRs, I'm not holding my breath...

@waldyrious waldyrious deleted the patch-1 branch July 31, 2016 20:03
@jab
Copy link
Owner

jab commented Aug 3, 2016

Right on @waldyrious. Thanks again for the PR.

@jab
Copy link
Owner

jab commented Jan 17, 2017

Hi @waldyrious, with this change I am no longer able to create new releases on PyPI. What a bummer.

twine upload ... fails with HTTPError: 400 Client Error: classifiers: 'License :: OSI Approved :: ISC License' is not a valid choice for this field for url: https://upload.pypi.org/legacy/.

Likewise, manually uploading a PKG-INFO file using the PyPI web interface fails with Error processing PKG-INFO data. Invalid classifier "License :: OSI Approved :: ISC License".

I was hoping to release 0.13.0 tonight but it looks like I'll have to back out this change to get past this error. Do you have any suggestions? Thanks in advance and hope to hear from you soon.

@waldyrious
Copy link
Contributor Author

I can't think of anything that would fix this immediately. This would require a change in PyPI via https://github.com/russianidiot/classifiers.py/pull/1 being merged, or something else which I'm unaware of.

Feel free to revert the 'License :: OSI Approved :: ISC License (ISCL)' --> 'License :: OSI Approved :: ISC License' change, of course, and let me know if you have any ideas about getting the PyPI issue fixed.

@waldyrious
Copy link
Contributor Author

If I'm reading this page correctly, https://pypi.python.org is still using the codebase that now lives at pypa/pypi-legacy (the new code, at pypa/warehouse, seems to power https://pypi.org -- which is still beta, it seems?). If that's indeed the case, then the line to be changed in this (and I suppose that would require a database update). So not looking good for a quick fix...

@jab
Copy link
Owner

jab commented Jan 19, 2017

Thanks, @waldyrious. I reverted that line so I could get the release out. I +1'd https://github.com/russianidiot/classifiers.py/pull/1 (and would be happy to do the same for any other issues / PRs that would show interest in getting this changed upstream).

@waldyrious
Copy link
Contributor Author

waldyrious commented Jun 20, 2020

Hey @jab, just to give you an update; I'm not sure why I assumed russianidiot/classifiers.py was the canonical source for the PyPI classifier (other than perhaps being the only instance of such a list on GitHub?), but the repo was never updated and has since been deleted. I just took another look and it seems that the canonical source is now hosted at https://github.com/pypa/trove-classifiers (since as recently as this April!), so I opened a PR (pypa/trove-classifiers#35) making the same change. Let's see if it sticks this time :)

@jab
Copy link
Owner

jab commented Jun 20, 2020

Thanks for the update, and hope you’ve been well. I actually switched bidict’s license to the MPLv2 a while ago, so no longer relevant here, but I do appreciate your following up!

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.

2 participants