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

Add Spanish translation #516

Merged
merged 5 commits into from
Oct 4, 2019
Merged

Add Spanish translation #516

merged 5 commits into from
Oct 4, 2019

Conversation

kbravh
Copy link
Contributor

@kbravh kbravh commented Oct 4, 2019

I have created the Spanish (es) translation folder and added all the necessary translations. I also added Spanish as an option in the Language dropdown in SettingsPanel.vue on line 83. This was not part of the translation readme (should it be? 🤷‍♂️).

Update index.js with prettier
@kbravh kbravh marked this pull request as ready for review October 4, 2019 16:26
@kbravh
Copy link
Contributor Author

kbravh commented Oct 4, 2019

As stated in #513, the translation instructions should mention running Prettier to appease Travis CI.

@yanfali
Copy link
Collaborator

yanfali commented Oct 4, 2019

@kbravh yeah, I think this is something I don't really pay attention to because I'm usually running yarn serve and it yells at me immediately when something is mal formatted. That's why we have travis 😄

src/i18n/es/index.js Outdated Show resolved Hide resolved
src/i18n/es/index.js Outdated Show resolved Hide resolved
Copy link
Collaborator

@yanfali yanfali left a comment

Choose a reason for hiding this comment

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

One bug, otherwise looks great. Let me get someone to double check the translations. Thanks so much for doing this.

src/i18n/index.js Outdated Show resolved Hide resolved
Copy link
Collaborator

@yanfali yanfali left a comment

Choose a reason for hiding this comment

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

LGTM, just waiting on some verification.

Copy link

@namelivia namelivia left a comment

Choose a reason for hiding this comment

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

I've suggested some small typo corrections and one possible word choice (plano vs layout vs distribución) to be discussed.

src/i18n/es/index.js Outdated Show resolved Hide resolved
src/i18n/es/index.js Outdated Show resolved Hide resolved
src/i18n/es/index.js Outdated Show resolved Hide resolved
src/i18n/es/tester.js Outdated Show resolved Hide resolved
src/i18n/es/index.js Outdated Show resolved Hide resolved
src/i18n/es/index.js Outdated Show resolved Hide resolved
src/i18n/es/index.js Outdated Show resolved Hide resolved
@kbravh
Copy link
Contributor Author

kbravh commented Oct 4, 2019

@namelivia I have updated the typos you found, and I also switched plano out for layout. Thanks for the review!

@yanfali
Copy link
Collaborator

yanfali commented Oct 4, 2019

@namelivia you can approve the PR if you agree. Then I'll merge. Thank you both so much

Copy link

@namelivia namelivia left a comment

Choose a reason for hiding this comment

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

Sorry I found one last typo I missed before! I'm pretty sure this is the last one.

src/i18n/es/index.js Outdated Show resolved Hide resolved
@yanfali
Copy link
Collaborator

yanfali commented Oct 4, 2019

OK, let me know guys when you want to merge.

Copy link

@namelivia namelivia left a comment

Choose a reason for hiding this comment

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

All ready now! Thank you very much!

@yanfali yanfali merged commit 6ef7023 into qmk:master Oct 4, 2019
@yanfali
Copy link
Collaborator

yanfali commented Oct 4, 2019

Thank you so much folks. This is much appreciated.

Copy link
Contributor

@johnny243 johnny243 left a comment

Choose a reason for hiding this comment

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

Nice work! Please see the suggested changes. Thanks!

EDIT: See #559

src/i18n/es/potato.js Show resolved Hide resolved
src/i18n/es/potato.js Show resolved Hide resolved
src/i18n/es/potato.js Show resolved Hide resolved
src/i18n/es/potato.js Show resolved Hide resolved
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.

4 participants