correct plural forms for week and millisecond in relative date phrases#1735
correct plural forms for week and millisecond in relative date phrases#1735sigmade wants to merge 7 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates Russian (ru) relative date phrase data so that week and millisecond use correct Russian grammatical-number forms (e.g., 2 недели, 2 миллисекунды) and adds regression coverage in the localisation phrase theory tests.
Changes:
- Extend
rurelative date phrase definitions forweekandmillisecondto use form maps (default/singular/dual) instead of a single plural form. - Add localisation theory test cases validating past/future phrasing for 1, 2, and 7 weeks/milliseconds in
ru.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tests/Humanizer.Tests/Localisation/LocalePhraseTheoryData.cs | Adds RU test cases for week/millisecond relative date phrases to verify corrected grammatical forms. |
| src/Humanizer/Locales/ru.yml | Updates RU relativeDate week and millisecond multiple.forms to support Russian singular/paucal/default selection. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
This PR adjusts Russian locale phrase data to use grammar-aware plural forms for relative date phrases so that counted expressions (e.g., “через 2 …”) use the correct Russian inflections.
Changes:
- Updated
ru.ymlrelative-date phrases forweekandmillisecondto providedefault/singular/dualforms instead of a single scalar form. - Updated the generated locale migration test to assert the presence of the Russian dual form for
weekin the phrase table.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/Humanizer.Tests/Localisation/GeneratedLocaleData/GeneratedLocaleDeRuPhraseMigrationTests.cs | Updates expectations to validate the added Russian dual form for weeks in the phrase table. |
| src/Humanizer/Locales/ru.yml | Adds grammar-aware plural forms for Russian relative-date weeks/milliseconds (past & future). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Assert.Equal("минуты", timeSpanMinute.Multiple?.Forms.Dual); | ||
| Assert.Equal("минут", timeSpanMinute.Multiple?.Forms.Default); | ||
|
|
||
| Assert.True(table.TryGetDatePhrase(TimeUnit.Week, Tense.Future, out var futureWeek)); |
| multiple: | ||
| forms: 'миллисекунд' | ||
| forms: | ||
| default: 'миллисекунд' | ||
| singular: 'миллисекунду' | ||
| dual: 'миллисекунды' | ||
| afterCount: 'назад' |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughFour Russian locale entries in ChangesRussian locale pluralization forms
SDK version update
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/Humanizer.Tests/Localisation/GeneratedLocaleData/GeneratedLocaleDeRuPhraseMigrationTests.cs (1)
47-50: ⚡ Quick winAdd assertions for all modified RU relative-date entries.
This assertion is correct, but this PR changed four RU phrase blocks. Adding checks for past week and past/future millisecond forms would close the regression gap with minimal effort.
Suggested test additions
Assert.True(table.TryGetDatePhrase(TimeUnit.Week, Tense.Future, out var futureWeek)); Assert.Equal("недель", futureWeek.Multiple?.Forms.Default); Assert.Equal("недели", futureWeek.Multiple?.Forms.Dual); + + Assert.True(table.TryGetDatePhrase(TimeUnit.Week, Tense.Past, out var pastWeek)); + Assert.Equal("недели", pastWeek.Multiple?.Forms.Dual); + + Assert.True(table.TryGetDatePhrase(TimeUnit.Millisecond, Tense.Future, out var futureMillisecond)); + Assert.Equal("миллисекунду", futureMillisecond.Multiple?.Forms.Singular); + Assert.Equal("миллисекунды", futureMillisecond.Multiple?.Forms.Dual); + Assert.Equal("миллисекунд", futureMillisecond.Multiple?.Forms.Default); + + Assert.True(table.TryGetDatePhrase(TimeUnit.Millisecond, Tense.Past, out var pastMillisecond)); + Assert.Equal("миллисекунду", pastMillisecond.Multiple?.Forms.Singular); + Assert.Equal("миллисекунды", pastMillisecond.Multiple?.Forms.Dual); + Assert.Equal("миллисекунд", pastMillisecond.Multiple?.Forms.Default);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Humanizer.Tests/Localisation/GeneratedLocaleData/GeneratedLocaleDeRuPhraseMigrationTests.cs` around lines 47 - 50, The test only asserts the RU future-week phrase (using table.TryGetDatePhrase(TimeUnit.Week, Tense.Future, out var futureWeek) and checks futureWeek.Multiple.Forms), but the PR changed four RU relative-date blocks; add assertions for the corresponding past-week entry and for both past and future millisecond entries: call table.TryGetDatePhrase(TimeUnit.Week, Tense.Past, out var pastWeek) and assert pastWeek.Multiple.Forms matches the expected "недель"/"недели" variants, and call table.TryGetDatePhrase(TimeUnit.Millisecond, Tense.Past, out var pastMs) and table.TryGetDatePhrase(TimeUnit.Millisecond, Tense.Future, out var futureMs) and assert their Single/Multiple Forms (or appropriate .Forms.Default/.Dual) equal the expected RU millisecond strings to cover all modified phrase blocks.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@tests/Humanizer.Tests/Localisation/GeneratedLocaleData/GeneratedLocaleDeRuPhraseMigrationTests.cs`:
- Around line 47-50: The test only asserts the RU future-week phrase (using
table.TryGetDatePhrase(TimeUnit.Week, Tense.Future, out var futureWeek) and
checks futureWeek.Multiple.Forms), but the PR changed four RU relative-date
blocks; add assertions for the corresponding past-week entry and for both past
and future millisecond entries: call table.TryGetDatePhrase(TimeUnit.Week,
Tense.Past, out var pastWeek) and assert pastWeek.Multiple.Forms matches the
expected "недель"/"недели" variants, and call
table.TryGetDatePhrase(TimeUnit.Millisecond, Tense.Past, out var pastMs) and
table.TryGetDatePhrase(TimeUnit.Millisecond, Tense.Future, out var futureMs) and
assert their Single/Multiple Forms (or appropriate .Forms.Default/.Dual) equal
the expected RU millisecond strings to cover all modified phrase blocks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3f5a3932-05a9-4b0c-a250-6cc132762dae
📒 Files selected for processing (2)
src/Humanizer/Locales/ru.ymltests/Humanizer.Tests/Localisation/GeneratedLocaleData/GeneratedLocaleDeRuPhraseMigrationTests.cs
|
@clairernovotny CodeQL was failing in actions/setup-dotnet before build/analyze because global.json pointed to the .NET 11 preview SDK version 11.0.100-preview.3, and that download was returning 404. I updated the SDK pin to the full published preview version 11.0.100-preview.5.26302.115 so actions/setup-dotnet can install the SDK successfully. This change is only to unblock the CI setup step the failure was unrelated to the RU locale changes. |
Here is a checklist you should tick through before submitting a pull request:
mainbranch (more info below)fixes #<the issue number>build.cmdorbuild.ps1and ensure there are no test failures