-
Notifications
You must be signed in to change notification settings - Fork 1.3k
1156 Update license in setup.cfg #1157
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
base: master
Are you sure you want to change the base?
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
I noticed the same issue and came here. I'm glad to see that this pull request has already proposed a fix — thank you for working on this. It seems that the addition of license information in this pull request follows an deprecated approach, according to the Python Packaging User Guide. It would be better to update it in accordance with PEP 639, like this: diff --git a/python/pyproject.toml b/python/pyproject.toml
index 4ec78b6..a646638 100644
--- a/python/pyproject.toml
+++ b/python/pyproject.toml
@@ -11,6 +11,8 @@ authors = [
description = "Unsupervised text tokenizer and detokenizer."
readme = "README.md"
requires-python = ">=3.9"
+license = "Apache-2.0"
+license-files = ["../LICENSE"]
urls = { "Homepage" = "https://github.com/google/sentencepiece" }
classifiers = [
"Programming Language :: Python :: 3",What do you think about this change? |
|
@tkng that also works. I think that is an even clear methods but have to read up on the path of License file |
|
@wagenrace I built a wheel file after applying the patch above, extracted its contents, and confirmed that the license file exists at |
|
@tkng Great, I updated it |
|
Sorry, when I tried building with Currently, the notation “../LICENSE” is allowed, but will not be allowed in the future... 😢 It seems better to have a symbolic link to the top level |
|
@tkng ironically, a deprecation warning of SetupTools is what brought me here in the first place. |
|
pypa/setuptools#2339
Personally, I think option 1 is clearer |
Pull Request Details
This will add the Apache-2.0 according to the SPDX standard and fixes #1156