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

Validate license fields #61

Merged
merged 2 commits into from May 6, 2015

Conversation

Projects
None yet
2 participants
@kemitchell
Contributor

kemitchell commented May 3, 2015

This PR adds new warnings about the "license" field:

  1. when there is no "license" field (including when "licenses" is used instead of "license")
  2. when the value of the "license" field is not a valid SPDX license expression string per kemitchell/spdx.js's implementation of the most recent standard draft, beta draft 0.98.

It addresses many concerns in npm/npm#6241 and npm/npm#4473.

I will be happy to make additional commits (or force-push commits) as needed to make this PR easy to merge. I've tried to conform to existing style, structure, and organization; please let me know what I've missed.

A few notes:

  1. spdx.js is a brand new package. The draft specification includes an ABNF grammar, but I found I had to make changes to support all of the examples in the draft and some common-sense variations. The test suite (embedded in the README) includes all the examples from the spec, plus a few more. Any help "torture testing" would be much appreciated.
  2. SPDX license expressions support complex conjunction and disjunction, e.g. (Apache-2.0 OR GPL-3.0+), which can describe multiply-licensed projects. It also supports LicenseRef-X syntax for licenses that don't yet have standard identifiers.

Why should anyone care? Behold:

Many thanks to the @maxogden and the dat team for making the above possible, and especially to @mafintosh for dat-npm. Tips of my hat to @hughsk, @wercker and @substack for inspiring conversation.

@othiym23 mentioned that I should also look at init-package-json. I have done some additional work on a function for rules-based estimation of which SPDX identifier was intended by an invalid license string that may be useful here or there or both. For now, I'm using it to make automated PRs to errant package authors, mostly to raise awareness.

@kemitchell

This comment has been minimized.

Contributor

kemitchell commented May 3, 2015

For the record, I did not attempt to pass the two unit tests that master was failing before I branched. As far as I can tell, neither failure turns on my additions for license field checking.

@kemitchell

This comment has been minimized.

Contributor

kemitchell commented May 6, 2015

I force-pushed use spdx@0.4, which supports version 2.0 of the expression syntax.

@othiym23 othiym23 merged commit 0401172 into npm:master May 6, 2015

1 check failed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
@kemitchell

This comment has been minimized.

Contributor

kemitchell commented May 6, 2015

Thanks, @othiym23!

@othiym23

This comment has been minimized.

Contributor

othiym23 commented May 6, 2015

Thank you for putting this together! I took the opportunity to fix the failing tests as well, in 336c480. Published as normalize-package-data@2.1.0. I'll see if anything must be done to land this in npm.

@kemitchell

This comment has been minimized.

Contributor

kemitchell commented May 6, 2015

@othiym23, if npm/npm#8179 is merged, we may want README to refer people to npm help 7 package.json instead of the doc for spdx.js.

@othiym23

This comment has been minimized.

Contributor

othiym23 commented May 6, 2015

It's fine to have normalize-package-json link to spdx.js's documentation instead of npm's, especially given that npm's documentation tends to change much more frequently than most of its dependencies. I think we're good here!

@kemitchell kemitchell deleted the kemitchell:validate-license-field branch May 6, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment