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

adds automated license checking to our pipeline #1357

Merged
merged 10 commits into from
Mar 7, 2019
Merged

adds automated license checking to our pipeline #1357

merged 10 commits into from
Mar 7, 2019

Conversation

ChrisBAshton
Copy link
Contributor

@ChrisBAshton ChrisBAshton commented Feb 27, 2019

Resolves N/A

Overall change: Adds apache2-license-checker to our pipeline to ensure we don't introduce dependencies that are incompatible with Apachev2.

Code changes:

  • Adds apache2-license-checker as a dependency
  • Adds a license-exceptions.json file which lists the dependencies the tool could not automatically check, and a reason why we think the dependencies are OK
  • Adds a call to apache2-license-checker in the pre-push step so we can't push incompatible branches.

Test notes

The apache2-license-checker should prevent us pushing if we add an Apache2-noncompliant dependency.

First of all git checkout -b testing-the-license-branch. Then try removing, for example, this from license-exceptions.json:

    "abab@*": {
      "reason": "Licensed under W3C 3-clause BSD license; https://github.com/jsdom/abab/blob/master/LICENSE.md"
    },

git commit -am "removed license exception - this commit should be fine".

Now git push origin testing-the-license-branch and the license checker script should run and fail ("abab is not compliant with Apache2" or similar message).

If you git push origin testing-the-license-branch --no-verify, it will force push (the --no-verify skips the verification step). You should then see it fail in Jenkins.


  • I have assigned myself to this PR and the corresponding issues
  • Tests added for new features
  • Test engineer approval
  • I have followed the merging checklist and this is ready to merge.

@ChrisBAshton ChrisBAshton self-assigned this Feb 27, 2019
Copy link
Contributor

@bcmn bcmn left a comment

Choose a reason for hiding this comment

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

This looks really good -- is it worth running on Jenkins/Travis as well, to avoid any sneaky --no-verifys which could undermine it?

Copy link
Contributor

@dr3 dr3 left a comment

Choose a reason for hiding this comment

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

I agree with ben, we should pop this as a pipeline step :)

@ChrisBAshton ChrisBAshton added the external blocked Blocked by an external dependency label Mar 1, 2019
@ChrisBAshton
Copy link
Contributor Author

@bbc/apache2-license-checker is currently a private package, despite the repo itself being open source: https://github.com/bbc/apache2-license-checker

Have raised bbc/apache2-license-checker#11.

@ChrisBAshton ChrisBAshton requested a review from dr3 March 4, 2019 15:19
package.json Outdated Show resolved Hide resolved
package.json Show resolved Hide resolved
dr3
dr3 previously approved these changes Mar 4, 2019
Copy link
Contributor

@dr3 dr3 left a comment

Choose a reason for hiding this comment

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

Im generally not a fan of running npx programatically, but LGTM :)

@dr3 dr3 removed the external blocked Blocked by an external dependency label Mar 4, 2019
ChrisBAshton added a commit that referenced this pull request Mar 4, 2019
depended on by #1357
@ChrisBAshton ChrisBAshton mentioned this pull request Mar 4, 2019
4 tasks
Copy link
Contributor

@pjlee11 pjlee11 left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@jamesbrumpton
Copy link
Contributor

👍

@dr3 dr3 merged commit f3fc149 into latest Mar 7, 2019
@dr3 dr3 deleted the license branch March 7, 2019 11:48
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.

5 participants