Copy proper app icon to /chrome/app/theme/mac/app.icns #653
Conversation
lib/util.js
Outdated
| // Each channel's app icons are stored in brave/app/theme/$channel/app.icns. | ||
| // With this copying, we don't need to modify chrome/BUILD.gn for this. | ||
| let channel = config.channel | ||
| if (config.debugBuild) |
There was a problem hiding this comment.
shouldn't this already be correct in config.channel? If not I think we should set it in config.js and not here
There was a problem hiding this comment.
Ah,, debug build uses different app icon with other four channel app icons.
There was a problem hiding this comment.
understood, what I'm saying is that the channel name (development) should be set in config.js, not here
There was a problem hiding this comment.
Ah, do you mean setting
developmenttoconfig.channelin debug build?
Oh, my previous comment was pending state :) I'll try to check this channel setting doesn't break anything.
There was a problem hiding this comment.
it should be checking the channel name here and doing any required icon name mapping, not checking debugBuild
There was a problem hiding this comment.
you don't necessarily have to set the channel to development, but it should be set to whatever makes sense for debugBuilds and then map from that to development for the icon path
There was a problem hiding this comment.
How about using development for channel in debug build?
I think it's fine because icon is stored under development folder/
There was a problem hiding this comment.
fine with me, but check with @mbacchi to be sure
There was a problem hiding this comment.
I think it would be fine because debug build is only used for local development build.
WDYT? @mbacchi
There was a problem hiding this comment.
Updated to set development to channel name.
How about merging this first and revisit here when @mbacchi comments something because mac release build is broken for now?
With this, we can avoid patching chrome/BUILD.gn for this.
simonhong
force-pushed
the
copy_mac_app_icon
branch
from
99d4ee7 to
4f7acc4
Compare
With this, we can avoid patching chrome/BUILD.gn for this.
issue: #652
Submitter Checklist:
git rebase -ito squash commits (if needed).Test Plan:
Reviewer Checklist: