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

License #7762

Merged
merged 86 commits into from
Jul 1, 2020
Merged

License #7762

merged 86 commits into from
Jul 1, 2020

Conversation

lionellloh
Copy link
Contributor

@lionellloh lionellloh commented Jun 17, 2020

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes? Here's an example.
  • Have you successfully run brew style with your changes locally?
  • Have you successfully run brew tests with your changes locally?

  • Added license DSL for formulae
  • Added audit check for license DSL for formulae.
  • Audit checks if a license info is a verified spdx-id against Homebrew/data/spdx.json
  • If --online flag is specified, and has github info, audit will do a comparison check to make sure what the formulae states matches what github API detects
  • Wrote tests to verify the behaviour for above

Copy link
Contributor

@jonchang jonchang left a comment

Choose a reason for hiding this comment

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

Incredible work so far!

Library/Homebrew/dev-cmd/audit.rb Outdated Show resolved Hide resolved
Library/Homebrew/dev-cmd/audit.rb Show resolved Hide resolved
Library/Homebrew/dev-cmd/audit.rb Outdated Show resolved Hide resolved
@lionellloh
Copy link
Contributor Author

Some more tweaks! Sorry about all the back and forward; it being quite a large PR means I keep catching new things each time. We're almost there, great work 👏

Please don't worry about it, I am learning quite a bit! I respect the quality control in the process and I would hope for nothing less as a user myself!

Library/Homebrew/test/dev-cmd/audit_spec.rb Outdated Show resolved Hide resolved
Library/Homebrew/test/dev-cmd/audit_spec.rb Outdated Show resolved Hide resolved
Library/Homebrew/utils/curl.rb Outdated Show resolved Hide resolved
Library/Homebrew/utils/github.rb Outdated Show resolved Hide resolved
@MikeMcQuaid
Copy link
Member

Tested this out locally and all worked as expected! The components order code has now been fixed if you wanted to add that into this PR?

@lionellloh
Copy link
Contributor Author

lionellloh commented Jun 30, 2020

Tested this out locally and all worked as expected! The components order code has now been fixed if you wanted to add that into this PR?

Thank you @MikeMcQuaid. Yes I think it's more efficient if I add that order enforcing logic into this PR since the context is all preserved here.

@MikeMcQuaid
Copy link
Member

Thank you @MikeMcQuaid. Yes I think it's more efficient if I add that order enforcing logic into this PR since the context is all preserved here.

Good call, I agree! Once that's done (and with a test) and CI is 🟢 : I think this is ready to merge! 🎉

@MikeMcQuaid MikeMcQuaid merged commit 8c8f2df into Homebrew:master Jul 1, 2020
@MikeMcQuaid
Copy link
Member

Thanks so much for your first contribution! Without people like you submitting PRs we couldn't run this project. You rock, @lionellloh!

Next work could be either the license deny list environment variable we discussed (in this repository) or adding some licenses to formulae in Homebrew/homebrew-core. For the latter: if you're bulk-updating a bunch of formulae instead of creating a giant PR (which will run a really long CI job we don't really care about) make one commit per-formula in your fork (https://github.com/MikeMcQuaid/dotfiles/blob/master/bin/git-commit-each) and link me to your branch.

@lionellloh lionellloh deleted the license branch July 1, 2020 13:22
@BrewTestBot BrewTestBot added the outdated PR was locked due to age label Dec 26, 2020
@Homebrew Homebrew locked as resolved and limited conversation to collaborators Dec 26, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants