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

Include docs about i18n #10588

Merged
merged 5 commits into from
Jul 22, 2022
Merged

Include docs about i18n #10588

merged 5 commits into from
Jul 22, 2022

Conversation

lidimayra
Copy link
Contributor

@lidimayra lidimayra commented Jul 22, 2022

The type of this PR is: Documentation

Description

We setup react-i18next during these changes, these docs contain instructions about the context, usage and next steps.

docs/i18n.md Outdated

## Context

Internationalization on Force was introduced as a hackathon project
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a tool that splits the lines like this? It doesn't make much sense for documentation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We were working on Notion and I guess I messed it up and I copied it from there, thanks for raising this!!

value
2. Once the PR is merged, it synchronizes with the third-party service
3. `en-US` is updated there and a PR is automatically opened to the repository on GitHub
4. The PR is merged by an Engineer so the project will include the updated locales
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the reason for requiring an engineer to merge this last PR? Couldn't it be automated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Technically, it can. We can discuss what would be the ideal approach.

Since the changes will affect the user experience, I guess we would like to at least check it before releasing it.

A change from Buy Now to Purchase, for example, would probably be fine in most cases, but depending on the content, if it changes too much the length of a string, for example, it could break the layout or look weird in components that didn't expect that kind of change.

We setup react-i18next during
[these changes](#9797), these docs
contain instructions about the context, usage and next steps.

Co-authored-by: Lidiane Taquehara <[email protected]>
Co-authored-by: Leandro Linares <[email protected]>
docs/i18n.md Outdated Show resolved Hide resolved
docs/i18n.md Show resolved Hide resolved
docs/i18n.md Show resolved Hide resolved
damassi
damassi previously approved these changes Jul 22, 2022
Copy link
Member

@damassi damassi left a comment

Choose a reason for hiding this comment

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

Docs look great @lidimayra 💯

One follow-up request here would be to move the locales folder under i18n/locales.

docs/i18n.md Outdated Show resolved Hide resolved
docs/i18n.md Outdated Show resolved Hide resolved
pvinis
pvinis previously approved these changes Jul 22, 2022
@lidimayra lidimayra dismissed stale reviews from pvinis and damassi via 0c2029e July 22, 2022 18:12
@lidimayra lidimayra merged commit 4a75bdc into main Jul 22, 2022
@lidimayra lidimayra deleted the i18n-docs branch July 22, 2022 18:34
@artsy-peril artsy-peril bot mentioned this pull request Jul 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants