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

Sort locales by language code #429

Closed
wants to merge 2 commits into from
Closed

Sort locales by language code #429

wants to merge 2 commits into from

Conversation

GreenLunar
Copy link
Contributor

I hope I did not break anything

@benlangfeld
Copy link
Member

The indenting is inconsistent here. Please follow the tab convention. You'll also need to say something to justify the change.

@GreenLunar
Copy link
Contributor Author

Does it look better?

@benlangfeld
Copy link
Member

You still have not given me any good reason to merge this.

@GreenLunar
Copy link
Contributor Author

Please assist me. I did sort translations according to language code and indent colons.

@benlangfeld
Copy link
Member

I understand what you did. I do not really understand why, however. Right now this is a change for the sake of a change. I need some better reason for this to be written down.

@GreenLunar
Copy link
Contributor Author

I did it so frivolous translators would not accidentally translate Candy into a language that is already available, because they thought that the languages are sorted already.

@linkmauve
Copy link
Contributor

I think it would be better to have one file per translation, extending the Candy.View.Translation object if it already exists (look at other files in the repository for examples), and let the filesystem handle the sorting for you.

Otherwise, if you decide to keep this version, please move the English translation where it should be, there is no reason to have an exception to keep it on top imo.

Also, please rebase your commits together once you are done and ready for a merge.

@benlangfeld
Copy link
Member

I think it would be better to have one file per translation

👍

@GreenLunar
Copy link
Contributor Author

See #479

@GreenLunar GreenLunar closed this Aug 10, 2016
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.

3 participants