-
Notifications
You must be signed in to change notification settings - Fork 862
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
Copy and package dev/beta channel png images #366
Conversation
When building dev/beta channel on Linux, the release png images were being packaged even though the alternate color images should have been present in the package. This corrects that behavior. Fixes brave/brave-browser#712
+ sources += [ | ||
+ "$branding_dir/product_logo_128_beta.png", | ||
+ "$branding_dir/product_logo_128_dev.png", | ||
+ "$branding_dir/product_logo_128_development.png", |
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.
how about handing nightly image also?
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.
I think we can avoid this patching and creating same copy target to our gn(ex, brave/BUILD.gn
) and adding it proper deps list like create_dist
target.
if [ "$CHANNEL" = "beta" ]; then | ||
icon_regex=".*product_logo_[0-9]\+_beta\." | ||
- elif [ "$CHANNEL" = "unstable" ]; then | ||
+ elif [ "$CHANNEL" = "unstable" -o "$CHANNEL" = "dev" ]; then |
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.
ditto
Thank you @simonhong for the recommendations, I've implemented the copy theme_files in our
I think this is due to the fact that the outputs directory ( If you have suggestions please let me know. For now I'm just adding the step that does the copy theme_files work to this PR. |
This reverts commit c7c8762.
In order to reduce unnecessary patching use a copy target in BUILD.gn.
branding_dir = "//chrome/app/theme/$branding_path_component" | ||
copy("theme_files") { | ||
visibility = [ ":*" ] | ||
if (!is_chrome_branded) { |
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.
This can be removed because this target is always used when !is_chrome_branded
.
In order to keep the commit history clean, I'm going to close this and open a new PR using the branch |
When building dev/beta channel on Linux, the release
png images were being packaged even though the
alternate color images should have been present in
the package. This corrects that behavior.
Fixes brave/brave-browser#712
Submitter Checklist:
git rebase -i
to squash commits (if needed).Test Plan:
Reviewer Checklist: