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

Rebranding password_manager_strings.grdp #6778

Merged
merged 2 commits into from
Nov 9, 2019

Conversation

simonhong
Copy link
Member

@simonhong simonhong commented Nov 6, 2019

Also, store generated management_strings.grpd to //brave/app.

fix #629

Submitter Checklist:

Test Plan:

Reviewer Checklist:

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

After-merge Checklist:

  • The associated issue milestone is set to the smallest version that the
    changes has landed on.
  • All relevant documentation has been updated.

@simonhong simonhong added this to the 0.74.x - Nightly milestone Nov 6, 2019
@simonhong simonhong self-assigned this Nov 6, 2019
@simonhong simonhong force-pushed the rebranding_password_manager_strings branch 2 times, most recently from e61eac7 to 4b125b1 Compare November 6, 2019 04:19
mkarolin
mkarolin previously approved these changes Nov 6, 2019
Copy link
Contributor

@mkarolin mkarolin left a comment

Choose a reason for hiding this comment

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

LGTM

lib/l10nUtil.js Outdated
@@ -40,7 +40,9 @@ const braveBookmarksBarStringsPartPath = path.resolve(path.join(srcDir, 'brave',

// src/components/components_strings.grd and any of its parts files.
const chromiumManagementStringsPartPath = path.resolve(path.join(srcDir, 'components', 'management_strings.grdp'))
const braveManagementStringsPartPath = path.resolve(path.join(srcDir, 'brave', 'chromium_src', 'components', 'management_strings.grdp'))
const braveManagementStringsPartPath = path.resolve(path.join(srcDir, 'brave', 'app', 'management_strings.grdp'))
Copy link
Contributor

Choose a reason for hiding this comment

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

these files should stay in components, they don't belong or make sense in app

Also, store generated management_strings.grpd to //brave/components
@simonhong simonhong force-pushed the rebranding_password_manager_strings branch from 838d506 to 02dcecb Compare November 8, 2019 06:25
bridiver
bridiver previously approved these changes Nov 8, 2019
lib/util.js Outdated
Comment on lines 157 to 161
autoGeneratedBraveToChromiumMapping[path.join(braveComponentsDir, 'components_brave_strings.grd')] = path.join(config.srcDir, 'components', 'components_brave_strings.grd')
autoGeneratedBraveToChromiumMapping[path.join(braveComponentsDir, 'components_strings.grd')] = path.join(config.srcDir, 'components', 'components_strings.grdp')
autoGeneratedBraveToChromiumMapping[path.join(braveComponentsDir, 'management_strings.grdp')] = path.join(config.srcDir, 'components', 'management_strings.grdp')
autoGeneratedBraveToChromiumMapping[path.join(braveComponentsDir, 'password_manager_strings.grdp')] = path.join(config.srcDir, 'components', 'password_manager_strings.grdp')
autoGeneratedBraveToChromiumMapping[path.join(braveComponentsDir, 'bookmark_bar_strings.grdp')] = path.join(config.srcDir, 'components', 'bookmark_bar_strings.grdp')
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: chromeComponentsDir is defined above.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed.

@simonhong simonhong merged commit 6c6df7f into master Nov 9, 2019
@simonhong simonhong deleted the rebranding_password_manager_strings branch November 9, 2019 23:11
mkarolin pushed a commit that referenced this pull request Dec 3, 2019
mkarolin pushed a commit that referenced this pull request Dec 3, 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.

Export Password creates file name labelled as Chrome Passwords
3 participants