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

Apply channel name to mac app name #208

Merged
merged 3 commits into from
Jul 3, 2018
Merged

Conversation

simonhong
Copy link
Member

@simonhong simonhong commented Jun 30, 2018

App name is changed to use different install dir for different channel.

Ex, beta channel -> Brave-Browser-Beta.app

Issue: brave/brave-browser#396, brave/brave-browser#10

Submitter Checklist:

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Ran git rebase -i to squash commits (if needed).
  • Tagged reviewers and labelled the pull request as needed.
  • Request a security/privacy review as needed.

Test Plan:

Execute Brave.dmg and check app name

Reviewer Checklist:

  • New files have MPL-2.0 license header.
  • Request a security/privacy review as needed.
  • Adequate test coverage exists to prevent regressions

@simonhong simonhong self-assigned this Jun 30, 2018
@simonhong simonhong requested a review from darkdh June 30, 2018 06:37
@simonhong simonhong force-pushed the apply_channel_to_mac_app_name branch from 3802368 to 5ddc23f Compare June 30, 2018 07:23
@simonhong
Copy link
Member Author

simonhong commented Jun 30, 2018

When I install various dmg by using this PR, only one app is visible in launchpad.
However, all installed different channel apps are visible in application folder.
I'm not sure this weird situation is related with this PR.
Also, installed app isn't started.

@simonhong simonhong changed the title Apply channel name to mac app name WIP: Apply channel name to mac app name Jun 30, 2018
@simonhong simonhong force-pushed the apply_channel_to_mac_app_name branch from 5ddc23f to 6ec78a8 Compare July 2, 2018 12:11
@simonhong simonhong changed the title WIP: Apply channel name to mac app name Apply channel name to mac app name Jul 2, 2018
@simonhong
Copy link
Member Author

I removed WIP label because above two problems are not related with this PR.
PTAL.

@@ -8,6 +8,16 @@ declare_args() {
_packaging_dir = "$root_out_dir/$brave_product_name Packaging"
keychain_db = getenv("HOME") + "/Library/Keychains/login.${mac_signing_keychain}-db"

_target_app_name = "Brave-Browser"
Copy link
Collaborator

Choose a reason for hiding this comment

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

does it make sense to create vars for "Brave-Browser" and "-Beta", etc.. in a gni file so it can be shared across platforms and stay consistent? I think the linux one I just reviewed could be combined as well

Copy link
Collaborator

@bridiver bridiver Jul 2, 2018

Choose a reason for hiding this comment

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

even if they use slightly different names/capitalization I think it would be good to have the logic for building the name in the same place

Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe that same gni file should create defines that we can use in the c++ code?

Copy link
Member Author

Choose a reason for hiding this comment

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

Deleted and logic in //brave/build/config.gni is used. PTAL.

simonhong added 3 commits July 3, 2018 12:33
Ex, beta channel -> Brave-Browser-Beta.app
Ex)Brave-Browser-Dev.dmg for dev channel dist build.
And use it in //brave/build/mac/BUILD.gn for applying channel to app and dmg
name.
@simonhong simonhong force-pushed the apply_channel_to_mac_app_name branch from 6ec78a8 to 4787658 Compare July 3, 2018 03:58
@bbondy bbondy merged commit eab720c into master Jul 3, 2018
@simonhong simonhong deleted the apply_channel_to_mac_app_name branch August 19, 2018 23:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants