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 script to check approval of a license. #318

Merged
merged 5 commits into from
Jan 18, 2016
Merged

Conversation

benbalter
Copy link
Contributor

A follow up to #316, this adds a quick script to check the approval of a given license to help with responding to issues proposing new licenses be added.

Example output:

➜  choosealicense.com git:(approval-script) script/check-approval mit
+--------------+-------------+
|        License: mit        |
+--------------+-------------+
| SPDX ID      | MIT         |
| SPDX Name    | MIT License |
| OSI approved | Yes         |
| FSF approved | No          |
| OD approved  | No          |
+--------------+-------------+
| Eligible     | Yes         |
+--------------+-------------+

Code search: https://github.com/search?q=mit+filename%3ALICENSE&type=Code
➜  choosealicense.com git:(approval-script) script/check-approval foobar
+--------------+---------+
|    License: foobar     |
+--------------+---------+
| SPDX ID      | Invalid |
| SPDX Name    | None    |
| OSI approved | No      |
| FSF approved | No      |
| OD approved  | No      |
+--------------+---------+
| Eligible     | No      |
+--------------+---------+

Code search: https://github.com/search?q=foobar+filename%3ALICENSE&type=Code

@mlinksva
Copy link
Contributor

This output is a bit odd:

|        License: mit        |
...
| FSF approved | No          |

@benbalter
Copy link
Contributor Author

This output is a bit odd:

Are you talking about the alignment? The colon in the title?

@mlinksva
Copy link
Contributor

Sorry for not being explicit. MIT is FSF approved. I realize not needed for eligibility calculation. But probably better to just not display if short-circuited (I haven't looked to see whether that's actually why the output is saying MIT isn't FSF approved).

@mlinksva
Copy link
Contributor

OK, it isn't short-circuited, but FSF calls it Expat. I don't know that it is really important that this script has correct output. Feel free to ignore.

benbalter added a commit that referenced this pull request Jan 18, 2016
Add script to check approval of a license.
@benbalter benbalter merged commit 58e9fae into gh-pages Jan 18, 2016
@benbalter benbalter deleted the approval-script branch January 18, 2016 22:14
@benbalter
Copy link
Contributor Author

Thanks @mlinksva. It's been helping me with the open issues, and glad to keep iterating on it as we find those types of edge cases (BSD 3-clause being another one).

wking added a commit to wking/choosealicense.com that referenced this pull request Jan 3, 2018
Avoid:

   $ ./script/check-approval MIT
   ./script/check-approval:55:in `<main>': undefined local variable or method `license_ids' for main:Object (NameError)

The license_ids line was added in e505eb8 (check if license is aleady
a license, 2016-01-18, github#318), but license_ids was removed from the
helper in b99e7ab (replace 'id' variables with 'spdx_lcase' to
minimize confusion, 2016-06-01, github#424).  This commit restores the old
code locally, since this script is the only consumer.
wking added a commit to wking/choosealicense.com that referenced this pull request Jan 3, 2018
The previous case-insensitive matching was removed in e5f46fa (test
required spdx-ids against data from spdx, 2016-05-25, github#418).  That
commit was designed [1] to allow case-sensitive matching as discussed
in [2].  But while I'm in favor of case-sensitive keys in spdx_list,
the case-sensitive match breaks script/check-approval which downcases
its argument since it was added in 8e56bb8 (add
script/check-approval, 2016-01-18, github#318).

[1]: github#418 (comment)
[2]: licensee/licensee#72
wking added a commit to wking/choosealicense.com that referenced this pull request Jan 3, 2018
The previous case-insensitive matching was removed in e5f46fa (test
required spdx-ids against data from spdx, 2016-05-25, github#418).  That
commit was designed [1] to allow case-sensitive matching as discussed
in [2].  But while I'm in favor of case-sensitive keys in spdx_list,
the case-sensitive match breaks script/check-approval which downcases
its argument since it was added in 8e56bb8 (add
script/check-approval, 2016-01-18, github#318).

There are more notes on SPDX's plans for case sensitivity in [3], so
we should see a clearer policy there soon.  I'm arguing for
case-sensitive *display* with optional case-insensitive matching.  I
am optimistic that the SPDX will at least agree not to register short
IDs that only differ by case, which is all we need to make this
case-insensitive match safe here.

[1]: github#418 (comment)
[2]: licensee/licensee#72
[3]: spdx/spdx-spec#63
wking added a commit to wking/choosealicense.com that referenced this pull request Jan 3, 2018
Avoid:

   $ ./script/check-approval MIT
   ./script/check-approval:55:in `<main>': undefined local variable or method `license_ids' for main:Object (NameError)

The license_ids line was added in e505eb8 (check if license is aleady
a license, 2016-01-18, github#318), but license_ids was removed from the
helper in b99e7ab (replace 'id' variables with 'spdx_lcase' to
minimize confusion, 2016-06-01, github#424).  This commit restores the old
code locally, since this script is the only consumer.
wking added a commit to wking/choosealicense.com that referenced this pull request Jan 3, 2018
Avoid:

   $ ./script/check-approval MIT
   ./script/check-approval:55:in `<main>': undefined local variable or method `license_ids' for main:Object (NameError)

The license_ids line was added in e505eb8 (check if license is aleady
a license, 2016-01-18, github#318), but license_ids was removed from the
helper in b99e7ab (replace 'id' variables with 'spdx_lcase' to
minimize confusion, 2016-06-01, github#424).  This commit restores the old
code locally, since this script is the only consumer.
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