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 brazilian portuguese translations #284

Conversation

arthurdenner
Copy link
Contributor

@arthurdenner arthurdenner commented Sep 3, 2017

Hi, guys.

Sorry for the wait to make this PR, I was very busy during the week.

I want to point that my last commit was a fix to the changes made in the turkish file after committing (idk what happened tbh, sorry about any mess that this may cause).

While translating, I created a function to make "one line texts" (I don't like to scroll horizontally xD). Hope that's OK. Also, during testing, I found three texts without translation available through files, so I changed them. Hope that's OK also.

Last but not least, sorry about the unformatted commits, I totally forgot about git cz (in 12 minutes, because my first commit was using it, smh).

yy: '%dy',
},
},
};
Copy link
Member

Choose a reason for hiding this comment

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

I'm not a language expert by any means. How different are Portuguese and Brazilian Portuguese? If they're very similar (UK English vs. US English), I think it probably makes more sense just to use "Portuguese".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In my opinion, Portugal and Brazilian Portuguese are not very similar like UK and US English, altough they share a lot of aspects, but vocabulary, phonetics and syntax are differ a bit. But yeah, maybe we could use the same translation.

Copy link
Member

Choose a reason for hiding this comment

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

Alright, if they're not too similar then this is totally fine!

@@ -15,4 +15,8 @@ export default [
code: 'tr',
name: 'Türkçe',
},
{
code: 'br',
Copy link
Member

Choose a reason for hiding this comment

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

Because this is not pure Brazilian, the code should be such pt-br, this also applies to the file name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was going to use pt-br, but the instructions says that we should use a two-letter code from here, so I used only br. Should I change?

Copy link
Member

Choose a reason for hiding this comment

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

Yea, those instructions should probably be changed. In this case, pt-br makes perfect sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, I will change the code and the file name as soon as I can. Thank you both for the suggestions and corrections.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changes made.

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.

All in all, this looks great. Thanks for spending the time to do this @arthurdenner, can't tell you how much we appreciate it :)

Thanks a million @lex111 and @Antoine38660 for taking a look at this 🙌 Please feel free to do a second pass through whenever you get the chance and if all looks good, we can merge this baby in.

@arthurdenner don't forget to add yourself as a contributor with yarn contributors:add! Feel free to do it as part of this PR or a separate one :)

@@ -206,7 +206,9 @@ export const RepositoryProfile = ({
? abbreviateNumber(repository.watchers_count)
: ' '}
</Text>
<Text style={styles.unitText}>Watchers</Text>
Copy link
Member

Choose a reason for hiding this comment

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

Thanks so much for noticing some stray translations <3

@@ -9,3 +9,5 @@ export const emojifyText = text => {
export const abbreviateNumber = count => {
return count >= 1000 ? (count / 1000).toFixed(1) + thousandUnit : count;
};

export const oneLineText = text => text.replace(/\s\s+/g, ' ');
Copy link
Member

Choose a reason for hiding this comment

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

👍

@@ -15,4 +15,8 @@ export default [
code: 'tr',
name: 'Türkçe',
},
{
code: 'ptBr',
Copy link
Member

Choose a reason for hiding this comment

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

The code must be written with a hyphen (pr-br); It will then be used in the library moment, and there it is necessary to write the language through a hyphen.

Copy link
Member

@andrewda andrewda left a comment

Choose a reason for hiding this comment

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

Looks good (besides @lex111's small request).

@arthurdenner
Copy link
Contributor Author

@housseindjirdeh I just committed updating the branch with the last master commit, changing what lex requested and, following your request, adding myself as contributor (to spare another PR). I guess everything is okay now? If not, let me know, please.

@andrewda
Copy link
Member

andrewda commented Sep 7, 2017

Looks all good except for the small merge conflicts. I can try to resolve.

@andrewda andrewda changed the title Add brazilian portuguese translations feat(translation): add brazilian portuguese translations Sep 7, 2017
Copy link
Member

@andrewda andrewda left a comment

Choose a reason for hiding this comment

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

LGTM

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.

Beautiful, thanks a million again @arthurdenner <3

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