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

[locale] Fix wrong calendar() output in 'be' localisation #4528

Merged
merged 4 commits into from
Apr 24, 2018

Conversation

mikitasolo
Copy link
Contributor

@mikitasolo mikitasolo commented Apr 2, 2018

fixes #4502
@demidov91, there was wrong regexp (I assume be localisation was copied from ru and Вв was not changed by Уу) and that caused wrong output of calendar() function (Неправильно брались падежи)

@mikitasolo mikitasolo changed the title [*] Fix wrong calendar() output in 'be' localisation [bugfix/feature/critical/new locale/locale/misc/tests/pkg] Fix wrong calendar() output in 'be' localisation Apr 2, 2018
@mikitasolo mikitasolo changed the title [bugfix/feature/critical/new locale/locale/misc/tests/pkg] Fix wrong calendar() output in 'be' localisation [locale] Fix wrong calendar() output in 'be' localisation Apr 2, 2018
@marwahaha
Copy link
Member

@Oire any thoughts ?

@DmitryScaletta
Copy link

DmitryScaletta commented Apr 6, 2018

As a native speaker, I can confirm that's correct.
And @SobakaSlava, maybe we should use Ууў instead of Уу?

@mikitasolo
Copy link
Contributor Author

mikitasolo commented Apr 7, 2018

@DmitryScaletta are there any cases of using ў? If yes, then i will add it.

@marwahaha
Copy link
Member

@DmitryScaletta @SobakaSlava shall I merge this?

@mikitasolo
Copy link
Contributor Author

@marwahaha Yes, sure

@marwahaha
Copy link
Member

@SobakaSlava - can you add a test?

@mikitasolo
Copy link
Contributor Author

@marwahaha actually it seems like the tests are already done for this case

@marwahaha
Copy link
Member

@SobakaSlava - to prevent this issue in the future, it'd be great to check in an additional test of the exact issue you have, to ensure that this change is what you want.

@marwahaha
Copy link
Member

marwahaha commented Apr 16, 2018

Ok, I tried pushing to your branch but I didn't have access.

Please add the following to src/test/locale/be.js:

test('calendar should format', function(assert) {
  assert.equal(moment('2018-04-13').calendar(moment('2018-04-16')), "У мінулую пятніцу ў 00:00", "calendar should handle day of week");
})

This was the old behavior, which I assume is unexpected.

test('calendar should format', function(assert) {
  assert.equal(moment('2018-04-13').calendar(moment('2018-04-16')), "У мінулую пятніца ў 00:00", "calendar should handle day of week");
})

@mikitasolo
Copy link
Contributor Author

@marwahaha sorry for a long wait. I've added the test.

@coveralls
Copy link

coveralls commented Apr 23, 2018

Coverage Status

Coverage increased (+0.07%) to 94.756% when pulling 0d5c531 on SobakaSlava:develop into aba77ec on moment:develop.

@itech-dzmitry
Copy link

Thanks for improvement! Excuse me for being not available during the conversation.

@mikitasolo
Copy link
Contributor Author

@marwahaha Ready to review

@marwahaha marwahaha merged commit 508f9ce into moment:develop Apr 24, 2018
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.

Wrong Belarusian translation: no cases.
5 participants