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

Update Linux app details to remove Google mention #1986

Merged
merged 1 commit into from
Mar 18, 2019

Conversation

bsclifton
Copy link
Member

@bsclifton bsclifton commented Mar 18, 2019

(NOTE: I haven't built/tested on Linux yet; will grab binaries from PR builder)

Please take a look at the files edited. Do these edits look correct? I want to make sure we update our Brand details (and also have relevant links for product issues / support), but I don't want to break any licensing terms

Fixes brave/brave-browser#392

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).
  • Verified that these changes build without errors on
    • Windows
    • macOS
    • Linux
  • Verified that these changes pass automated tests (npm test brave_unit_tests && npm test brave_browser_tests) on
    • Windows
    • macOS
    • Linux
  • Verified that all lint errors/warnings are resolved (npm run lint)
  • Ran git rebase master (if needed).
  • 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.
  • Add appropriate QA labels (QA/Yes or QA/No) to include the closed issue in milestone

Test Plan:

See brave/brave-browser#392

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

@bsclifton
Copy link
Member Author

bsclifton commented Mar 18, 2019

There's a 3rd file at src/chrome/installer/linux/common/google-chrome/google-chrome.info which has more details, but that looks to be Chrome specific (ex: not Chromium). I don't think it's used

@bsclifton bsclifton force-pushed the update-linux-app-details branch from ae238e0 to 1fb6de4 Compare March 18, 2019 07:09
<id>@@PACKAGE@@.desktop</id>
- <update_contact>chromium-dev@chromium.org</update_contact>
+ <update_contact>support@brave.com</update_contact>
<metadata_license>CC0-1.0</metadata_license>
Copy link
Member Author

Choose a reason for hiding this comment

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

NOTE: this metadata_license is for the actual XML file itself (not the project). More info available at https://www.freedesktop.org/software/appstream/docs/chap-Quickstart.html

@simonhong
Copy link
Member

Tested on my local machine and manpage patch works as expected.
Regarding to appdata, we don't use chrome.appdata.xml.template.
Instead, we override chromium-browser.appdata.xml in chrome/installer/linux/BUILD.gn.
So, how about overriding manpage.1.in instead of patching like appdata.xml did?

@bsclifton
Copy link
Member Author

@simonhong good call- will update to do that 😄

@bsclifton bsclifton force-pushed the update-linux-app-details branch from 1fb6de4 to 6d59be6 Compare March 18, 2019 17:02
@bsclifton bsclifton force-pushed the update-linux-app-details branch from 6d59be6 to 726633b Compare March 18, 2019 17:04
@bsclifton
Copy link
Member Author

@simonhong @iefremov updated 👍

Copy link
Member

@simonhong simonhong left a comment

Choose a reason for hiding this comment

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

LGTM

@bsclifton
Copy link
Member Author

Verified this looks great by downloading the binaries provided by PR builder (https://s3-us-west-2.amazonaws.com/brave-jenkins-build-artifacts/brave-browser-build-pr/update-linux-app-details/3/brave-browser-dev_0.64.4_amd64.deb)

Before

Screen Shot 2019-03-18 at 1 56 16 PM

Screen Shot 2019-03-18 at 1 56 35 PM

After

Screen Shot 2019-03-18 at 1 56 48 PM

Screen Shot 2019-03-18 at 1 57 05 PM

@bsclifton bsclifton merged commit a9e9ae9 into master Mar 18, 2019
@bsclifton bsclifton deleted the update-linux-app-details branch March 18, 2019 21:02
brave-builds pushed a commit that referenced this pull request Mar 18, 2019
brave-builds pushed a commit that referenced this pull request Mar 18, 2019
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.

Linux command line help needs updating for brave specific details
2 participants