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

Keep the translation missing message when default is not nil #389

Merged
merged 1 commit into from Nov 3, 2017

Conversation

@rafaelfranca
Copy link
Contributor

@rafaelfranca rafaelfranca commented Oct 26, 2017

When either the key and the all the defaults have missing translation we should be showing the original key in the translation missing message.

Before this commit we were showing a translation missing error in the "no key" key.

This commit fixes a regression introduced in #387.

@radar
Copy link
Collaborator

@radar radar commented Nov 1, 2017

Thank you @rafaelfranca and @Edouard-chin ❤️

This patch looks good to me. Could you please rebase this PR on top of the latest master? Master contains #390, which should fix some of your broken Travis tests (I hope 🤞 )

@rafaelfranca rafaelfranca force-pushed the rafaelfranca:fix-default-regression branch Nov 2, 2017
@rafaelfranca
Copy link
Contributor Author

@rafaelfranca rafaelfranca commented Nov 2, 2017

Done!

@radar
Copy link
Collaborator

@radar radar commented Nov 2, 2017

Thanks! Travis is still broken though. Let me take a look... 🤔

@rafaelfranca
Copy link
Contributor Author

@rafaelfranca rafaelfranca commented Nov 2, 2017

Looks related to Digest no being required. Maybe it was required by one of the files i18n require but now it is not anymore.

When either the key and the all the defaults have missing translation we
should be showing the original key in the translation missing message.

Before this commit we were showing a translation missing error in the
"no key" key.

This commit fixes a regression introduced in #387.
@radar radar force-pushed the rafaelfranca:fix-default-regression branch to 4104295 Nov 2, 2017
@radar
Copy link
Collaborator

@radar radar commented Nov 2, 2017

I've just rebased your branch on top of master. This should fix the Travis build and then we should be good to :shipit:!

@rafaelfranca
Copy link
Contributor Author

@rafaelfranca rafaelfranca commented Nov 3, 2017

Thanks! It is green now.

@radar
Copy link
Collaborator

@radar radar commented Nov 3, 2017

It sure is! ❤️

@radar radar merged commit 86cc086 into ruby-i18n:master Nov 3, 2017
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@radar
Copy link
Collaborator

@radar radar commented Nov 3, 2017

v0.9.1 has just been released which contains this fix, as well as #390. Thank you for the attentiveness to this PR.

@rafaelfranca
Copy link
Contributor Author

@rafaelfranca rafaelfranca commented Nov 3, 2017

Thanks!

@rafaelfranca rafaelfranca deleted the rafaelfranca:fix-default-regression branch Nov 3, 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
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants