-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Auto-generate NTP backgrounds LICENSE file #8032
Conversation
3597270
to
b18443c
Compare
b18443c
to
5f450bc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left notes - LMK what you think! 😄
5f450bc
to
cf023d9
Compare
cf023d9
to
4a11656
Compare
The only test failure is this one:
which is a known problem. @bsclifton @bridiver Can one of you re-review so that I can merge this PR? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
++
const authorLink = getValidatedComponentField(component, 'link') | ||
const originalUrl = getValidatedComponentField(component, 'originalUrl') | ||
const license = getValidatedComponentField(component, 'license') | ||
if ((license != 'used with permission') && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @fmarier better if this could be case insensitive. had to workaround in brave/brave-core#4864
Fixes #7460 along with brave/brave-core#4481.
Submitter Checklist:
git rebase master
(if needed).git rebase -i
to squash commits (if needed).added to
scripts/audit.js
.Test Plan:
npm run build Release
Look at the "Background images" entry in
brave://credits
. It should contain URLs and licenses for all 16 background images.Reviewer Checklist:
After-merge Checklist:
changes has landed on.