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.

Sign up for GitHub

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

Jump to bottom

Add missing dependencies to brave://credits #3899

Merged
merged 13 commits into from
Nov 9, 2019
Merged

Add missing dependencies to brave://credits #3899

merged 13 commits into from
Nov 9, 2019

Conversation

fmarier
Copy link
Member

@fmarier fmarier commented Nov 7, 2019 •
edited
Loading

Fixes brave/brave-browser#6809.

(Looking at the commits individually might be easier than looking at the big diff.)

Submitter Checklist:

Test Plan:

Open brave://credits and ensure that these entries are present:

  • autoplay-whitelist
  • Background images
  • Bluebird
  • boto
  • Brave Sync
  • Bulgarian adblock list
  • EasyList
  • Muli (listed twice)
  • PROCHLO
  • Tor
  • TweetNaCl
  • ZXing

For each of these, check that:

  • the license link should display the license
  • the homepage link should work

Reviewer Checklist:

  • New files have MPL-2.0 license header.
  • Request a security/privacy review as needed.
  • Adequate test coverage exists to prevent regressions
  • Verify test plan is specified in PR before merging to source

After-merge Checklist:

  • The associated issue milestone is set to the smallest version that the
    changes has landed on.
  • All relevant documentation has been updated.

@fmarier fmarier self-assigned this Nov 7, 2019
@fmarier fmarier changed the title Add missing third-parties to brave://credits Add missing dependencies to brave://credits Nov 7, 2019
@fmarier fmarier force-pushed the francois-add-missing-licenses-6809 branch from 520cd96 to d15e4aa Compare November 7, 2019 01:15
tmancey
tmancey approved these changes Nov 7, 2019
View reviewed changes
Copy link
Collaborator

@tmancey tmancey left a comment

Choose a reason for hiding this comment

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

LGTM++

@fmarier fmarier force-pushed the francois-add-missing-licenses-6809 branch from d15e4aa to f5a5c39 Compare November 7, 2019 20:12
@fmarier fmarier force-pushed the francois-add-missing-licenses-6809 branch from f5a5c39 to 622b240 Compare November 7, 2019 22:39
@fmarier fmarier requested a review from bridiver November 8, 2019 00:36
@fmarier fmarier force-pushed the francois-add-missing-licenses-6809 branch from 622b240 to 7e979b8 Compare November 8, 2019 00:48
@fmarier
Add adblock_bg license after confirming with author by email
6128531
@fmarier
Add license notices for node modules
bbed244 
This includes all of the dependencies (excluding devDependencies)
in brave-browser and brave-core package.json.
@fmarier
Remove unnecessary copies of the MPL-2.0
8ab12a5 
These files were only needed before these separate projects got
merged into the brave-core repo.
@fmarier
Add missing third-party code to brave://credits (fixes brave/brave-br…
c91993d 
…owser#6809)
@fmarier fmarier force-pushed the francois-add-missing-licenses-6809 branch from 7e979b8 to c91993d Compare November 8, 2019 00:52
bsclifton
bsclifton approved these changes Nov 8, 2019
View reviewed changes
Copy link
Member

@bsclifton bsclifton left a comment

Choose a reason for hiding this comment

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

Great job wrangling all these licenses together! 😄 Definitely easy to read on a commit/commit basis

@bsclifton bsclifton added this to the 0.74.x - Nightly milestone Nov 8, 2019
@kylehickinson kylehickinson removed their request for review November 8, 2019 15:07
@fmarier
Copy link
Member Author

fmarier commented Nov 8, 2019

With the exception of https://ci.brave.com/job/brave-browser-build-pr/job/francois-add-missing-licenses-6809/4/execution/node/478/log/ (which @linhkikuchi points out is test-install), nothing is read in CI.

The Mac build timed out however: https://ci.brave.com/job/brave-browser-build-pr/job/francois-add-missing-licenses-6809/4/execution/node/708/log/

@fmarier fmarier added CI/skip-android Do not run CI builds for Android CI/skip-ios Do not run CI builds for iOS CI/skip-linux labels Nov 8, 2019
@bsclifton
Copy link
Member

Verified this works (and builds) great on macOS - CI had some complications; I'm 👍on merging since it works great here 😄

@fmarier fmarier merged commit 3bd396d into master Nov 9, 2019
@fmarier fmarier deleted the francois-add-missing-licenses-6809 branch November 9, 2019 01:16
This was referenced Nov 9, 2019
Closed
Merged
Merged
Merged
@GeetaSarvadnya GeetaSarvadnya mentioned this pull request Nov 11, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bsclifton bsclifton bsclifton approved these changes

@tmancey tmancey tmancey approved these changes

@bridiver bridiver bridiver approved these changes

@emerick emerick Awaiting requested review from emerick

@mkarolin mkarolin Awaiting requested review from mkarolin

@gdregalo gdregalo Awaiting requested review from gdregalo

@NejcZdovc NejcZdovc Awaiting requested review from NejcZdovc

Assignees

@fmarier fmarier

Labels
CI/skip-android Do not run CI builds for Android CI/skip-ios Do not run CI builds for iOS
Projects
None yet
Milestone
1.3.x - Release
Development

Successfully merging this pull request may close these issues.

Add missing dependencies to brave://credits
4 participants
@fmarier @bsclifton @bridiver @tmancey