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

feat(translation): add pt translation #440

Merged
merged 11 commits into from Oct 8, 2017
Merged

feat(translation): add pt translation #440

merged 11 commits into from Oct 8, 2017

Conversation

Jpfonseca
Copy link
Contributor

@Jpfonseca Jpfonseca commented Oct 6, 2017

Added pt-pt as asked in #439 .

@lex111
Copy link
Member

lex111 commented Oct 6, 2017

@Jpfonseca It's just Portuguese, right? If so, then leave just pt.

And please read the instruction for adding a new translation. Thanks in advance!

@Jpfonseca
Copy link
Contributor Author

Jpfonseca commented Oct 6, 2017

@lex111 there are 2 types of portuguese : pt-pt and pt-br. That's why I kept the name of the file

Copy link
Member

@housseindjirdeh housseindjirdeh left a comment

Choose a reason for hiding this comment

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

Agreed, we already have Brazilian Portuguese so it only makes sense to specify the Portuguese variation :)

Thank you, this looks great to me. Feel free to do another pass through @lex111 and if it looks good let's merge this baby in

@@ -381,6 +381,13 @@
"code",
"bug"
]
},
{
"login": "Jpfonseca",
Copy link
Member

Choose a reason for hiding this comment

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

🎉

@lex111
Copy link
Member

lex111 commented Oct 7, 2017

@Jpfonseca I understood, but the fact is that we change the original name of the locale, making it compatible with moment library, so we need the name of the locale to match the name of the locale in moment:

image

Therefore, please change the code with pt-pt everywhere to just pt.

P.S. @housseindjirdeh how correctly to describe this in the instruction?

@Jpfonseca
Copy link
Contributor Author

Jpfonseca commented Oct 7, 2017

@lex111 Sorry that i didn't do that before .
I totally understand the situation, and agree with you.

Copy link
Member

@lex111 lex111 left a comment

Choose a reason for hiding this comment

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

Remained to do the last small edit.

@@ -20,6 +20,11 @@ export default [
name: 'Türkçe',
},
{
code: 'pt-pt',
Copy link
Member

Choose a reason for hiding this comment

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

pt instead pt-pt.

@andrewda andrewda changed the title Added pt-pt language file feat(translation): add pt translation Oct 7, 2017
@housseindjirdeh
Copy link
Member

Cheers thank you @lex111 and sorry I didn't realize that earlier. Added that step to our translations instructions.

@Jpfonseca
Copy link
Contributor Author

Jpfonseca commented Oct 7, 2017

@lex111 I think you can merge it now ..
Sorry for the trouble.

@housseindjirdeh
Copy link
Member

housseindjirdeh commented Oct 8, 2017

@Jpfonseca No trouble at all! If anything, we need to apologize for not being super clear in the docs and having to tell you to change it up slghtly

Thank you for this, really means a lot to take the time to get all the translations in ❤️

@housseindjirdeh housseindjirdeh merged commit 94e5933 into gitpoint:master Oct 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants