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

docs(i18n): add 'meaning' to i18n README #9787

Merged
merged 2 commits into from
Oct 8, 2019
Merged

Conversation

exterkamp
Copy link
Member

Summary
I noticed, upon reviewing the l10n plan for DevTools, that we never mention our divergence from message.json format (read: "meanings").

lighthouse-core/lib/i18n/README.md Outdated Show resolved Hide resolved
@@ -202,6 +202,7 @@ CTC is a name that is distinct and identifies this as the Chrome translation for
"name": {
"message": "Message text, with optional placeholders.",
"description": "Translator-aimed description of the message.",
"meaning": "Description given when a message is duplicated, in order to give context to the message.",
Copy link
Collaborator

Choose a reason for hiding this comment

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

isn't this just a copy of description for us? seems like this doc is a good place to diverge from the chrome docs if we're doing something different :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup. Just having a meaning in our spec is the divergence. Should there be more context for it?

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.

+1 to @patrickhulce's comments, but I'm a little confused by the purpose of this. Without an explanation of what meaning is, having it in the example (especially when the example is already labelled "From the Chrome Docs") doesn't really seem to clarify anything about it. Maybe it needs its own little subsection somewhere in here?

@exterkamp
Copy link
Member Author

  • removed the "from the chrome docs" since it isn't anymore
  • reworded top message
  • added more explanation to the "meaning" attribute

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.

LGTM

```json
{
"name": {
"message": "Message text, with optional placeholders.",
"description": "Translator-aimed description of the message.",
"meaning": "Description given when a message is duplicated, in order to give context to the message. Lighthouse uses a copy of the description for this.",
Copy link
Member

Choose a reason for hiding this comment

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

ha, I guess the example fields documenting themselves works :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants