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

core(fcp-3g): remove unused i18n for LR compatibility #7103

Merged
merged 4 commits into from
Jan 29, 2019
Merged

Conversation

exterkamp
Copy link
Member

@exterkamp exterkamp commented Jan 29, 2019

Summary
Drop the require.resolveto find the i18n strings in exchange for an inlined UIStrings so that browserify can find them and the audit can be run in Lightrider.

Copy link
Collaborator

@patrickhulce patrickhulce left a comment

Choose a reason for hiding this comment

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

Oh my b! I had thought browserify would just do the right thing here :/

The strings aren't used in the UI AFAIK. Do we even need to i18n them? I didn't create the extra copy for this reason.

Copy link
Member

@brendankenny brendankenny left a comment

Choose a reason for hiding this comment

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

I had thought browserify would just do the right thing here :/

yeah, weird that __filename gets resolved without complaint and brfs will handle require.resolve if nested inside a fs.readFileSync() call, but just a plain old require.resolve won't be taken care of.

The strings aren't used in the UI AFAIK. Do we even need to i18n them?

@paulirish is probably the only one to answer this as he seems to be the only one who really knows how this audit is being used :)

@paulirish
Copy link
Member

We don't need to i18n them. This will only be pulled programmatically.

Similar to metrics and screenshot-thumbnails which don't get i18n'd. So we can just leave them unadorned as well.

@brendankenny brendankenny changed the title core(fcp 3g): make i18n self contained for LR compatibility core(fcp-3g): remove unused i18n for LR compatibility Jan 29, 2019
@brendankenny brendankenny merged commit 9658610 into master Jan 29, 2019
@brendankenny brendankenny deleted the i18n-fcp-3g branch January 29, 2019 20:21
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.

None yet

4 participants