Skip to content

Conversation

@aduth
Copy link
Member

@aduth aduth commented Sep 6, 2017

This pull request seeks to resolve invalid usage of i18n functions where string extraction by static analysis would be unable to retrieve the referenced string argument. There was one instance of this in the embed block.

Further, the behavior of the embed block was such that it would attempt to translate titles for services such as YouTube, etc. Since these are brands, they are not intended to be translatable.

Finally, in order to prevent against future issues, I've introduced new lint rules for restricted syntax which will capture these errors during CI.

Testing instructions:

Verify that generating gettext strings includes the "Embed" string, and not any of the other embed services.

npm run test-client

Try locally to update the invocation of __, _n, _x, or _nx to include an argument which is not either a string or concatenated string (see #2655), and confirm that linting fails:

npm run lint

@aduth aduth added [Feature] Blocks Overall functionality of blocks Internationalization (i18n) Issues or PRs related to internationalization efforts labels Sep 6, 2017
@aduth aduth force-pushed the fix/embed-i18n-args branch from f47cce9 to 81ad676 Compare September 7, 2017 12:51
@codecov
Copy link

codecov bot commented Sep 7, 2017

Codecov Report

Merging #2687 into master will increase coverage by 1.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2687      +/-   ##
==========================================
+ Coverage   33.15%   34.17%   +1.01%     
==========================================
  Files         182      185       +3     
  Lines        5540     5653     +113     
  Branches      963      994      +31     
==========================================
+ Hits         1837     1932      +95     
- Misses       3136     3153      +17     
- Partials      567      568       +1
Impacted Files Coverage Δ
blocks/library/embed/index.js 45.55% <ø> (ø) ⬆️
editor/modes/visual-editor/block.js 0% <0%> (ø) ⬆️
editor/modes/visual-editor/block-crash-boundary.js 0% <0%> (ø) ⬆️
utils/focus/test/utils/create-element.js 100% <0%> (ø)
utils/focus/focusable.js 93.33% <0%> (ø)
utils/focus/tabbable.js 100% <0%> (ø)
blocks/api/validation.js 95.23% <0%> (+0.07%) ⬆️
blocks/api/serializer.js 98% <0%> (+0.08%) ⬆️
blocks/api/parser.js 98.3% <0%> (+0.15%) ⬆️
editor/inserter/menu.js 49.61% <0%> (+1.2%) ⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 344e700...81ad676. Read the comment docs.

@aduth aduth merged commit 8afb094 into master Sep 8, 2017
@aduth aduth deleted the fix/embed-i18n-args branch September 8, 2017 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Feature] Blocks Overall functionality of blocks Internationalization (i18n) Issues or PRs related to internationalization efforts

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants