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

[WIP] Change strategy to add Weeks. Add Week property #807

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

AKTheKnight
Copy link
Contributor

@AKTheKnight AKTheKnight commented Apr 1, 2019

fixes #765

This also breaks lots of tests that are missing the property, I'll work on fixing them

This also breaks lots of tests that are missing the property, not sure what we do
JUst 85 localized tests left
@@ -88,8 +88,8 @@ public void HoursFromNowNotTomorrow(int hours, string expected)

[Theory]
[InlineData(1, "yesterday")]
[InlineData(10, "10 days ago")]
[InlineData(27, "27 days ago")]
Copy link
Collaborator

Choose a reason for hiding this comment

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

given this test passes in TimeUnit.Day, why would it result in week/weeks

also 10 days is no "on week"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, I think this is going to be a culture thing and then a question of how precise people would like humanizer to be.

7 days would be "a week ago", and in fact if someone asked me about something 8 days ago I might also accept "a week ago".

Perhaps I should look at making this only provide weeks if the value is one of 7,14,21,28. And then fall backs to months as expected after that 🤔

@SimonCropp SimonCropp changed the title [WIP] Fix #765. Change strategy to add Weeks. Add Week property [WIP] Change strategy to add Weeks. Add Week property Feb 16, 2024
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.

DateTime Humanizer : returns hours, days, months, but not weeks
2 participants