Raise errors on long i18n message ids #1438
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
33 tasks
|
Ok @toolness, I just added ID tags to fix all of our msgid-length warnings! I decided to set the threshold to 175 characters, as this seemed to be a good break point in the list of messages that we got. |
toolness
commented
May 15, 2020
Comment on lines
+20
to
+22
| t( | ||
| "norent.tweetTemplateForSharingNoRent" | ||
| )`No idea how you'll pay rent this month? Tell your landlord with norent.org from @JustFixNYC. This free tool sends a certified letter informing them of your protections. Join the #cancelrent movement at norent.org.` |
There was a problem hiding this comment.
Oh super dope, I didn't realize there was a way to manually specify an ID for these non-JSX thingys!
|
Awesome thanks! I'll fix up the letter builder ones and then activate HARDFAIL and merge! |
|
oh wow, thanks, I didn't actually realize you addressed all the warnings, not just the ones in primary pages! |
toolness
changed the title
toolness
added a commit
that referenced
this pull request
May 19, 2020
So #1438 helped us reduce the size of our browser code footprint, but Crowdin didn't like it much, because it presented the message ID as the string to be translated, rather than the English version of the actual string. Fortunately, after talking to Crowdin support, it looks like this is remedied with a simple header on the PO file, which this PR adds.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
From a Slack conversation:
This adds functionality to our localebuilder tool which
logs warnings (which we could convert to errors in a future PR)raises errors whenever lingui message catalog IDs are longer than100175 characters, indicating that significant space might be saved if we shorten them. We can always change the limit if we want, too.Here's a screenshot of its output:
Since localebuilder is run in production and the output is colorized, this PR also moves
chalkfrom ourdevDependenciestodependencies(it's already ultimately in there anyways due to various third-party packages independenciesalready needing it).