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

Fixed issue 238 - Change DefaultDateTimeHumanizeStrategy to turn 60 min ... #278

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/Humanizer.Tests/DateHumanizeDefaultStrategyTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,9 @@ public void SecondsFromNow(int seconds, string expected)
[InlineData(1, "a minute ago")]
[InlineData(10, "10 minutes ago")]
[InlineData(44, "44 minutes ago")]
[InlineData(45, "an hour ago")]
[InlineData(45, "45 minutes ago")]
[InlineData(59, "59 minutes ago")]
[InlineData(60, "an hour ago")]
[InlineData(119, "an hour ago")]
[InlineData(120, "2 hours ago")]
public void MinutesAgo(int minutes, string expected)
Expand All @@ -47,7 +49,7 @@ public void MinutesAgo(int minutes, string expected)
[InlineData(1, "a minute from now")]
[InlineData(10, "10 minutes from now")]
[InlineData(44, "44 minutes from now")]
[InlineData(45, "an hour from now")]
[InlineData(45, "45 minutes from now")]
[InlineData(119, "an hour from now")]
[InlineData(120, "2 hours from now")]
public void MinutesFromNow(int minutes, string expected)
Expand Down
2 changes: 1 addition & 1 deletion src/Humanizer.Tests/Localisation/da/DateHumanizeTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public void MinutesAgo(int minutes, string expected)
[InlineData(1, "et minut fra nu")]
[InlineData(10, "10 minutter fra nu")]
[InlineData(44, "44 minutter fra nu")]
[InlineData(45, "en time fra nu")]
[InlineData(45, "45 minutter fra nu")]
Copy link
Member

Choose a reason for hiding this comment

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

Add 60 minutes for all languages please as that's currently missing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cool will do that today

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sorry MehdiK didn't get that. I have added the 60 minutes to the strategy it self hence it now works for all languages.

Copy link
Member

Choose a reason for hiding this comment

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

Correct. Previously the logic was to return "an hour" for 45 minutes and we had test coverage for that in all locales. The code is now changed/fixed to return "one hour" for 60 minutes but we no longer have unit tests for that in any of the other locales. Just need to fix the tests to reflect the change. Thanks.

[InlineData(119, "en time fra nu")]
[InlineData(120, "2 timer fra nu")]
public void MinutesFromNow(int minutes, string expected)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public void MinutesAgo(int minutes, string expected)
[InlineData(1, "ett minutt fra nå")]
[InlineData(10, "10 minutter fra nå")]
[InlineData(44, "44 minutter fra nå")]
[InlineData(45, "en time fra nå")]
[InlineData(45, "45 minutter fra nå")]
[InlineData(119, "en time fra nå")]
[InlineData(120, "2 timer fra nå")]
public void MinutesFromNow(int minutes, string expected)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ public string Humanize(DateTime input, DateTime comparisonBase)
if (ts.TotalMinutes < 45)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This condition is not needed any more, as it also included in the next condition < 60.
We should remove it.

return Configurator.Formatter.DateHumanize(TimeUnit.Minute, tense, ts.Minutes);

if (ts.TotalMinutes < 60)
return Configurator.Formatter.DateHumanize(TimeUnit.Minute, tense, ts.Minutes);

if (ts.TotalMinutes < 90)
return Configurator.Formatter.DateHumanize(TimeUnit.Hour, tense, 1);

Expand Down
8 changes: 5 additions & 3 deletions src/Humanizer/FluentDate/On.Days.cs
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
using System;

using System;

namespace Humanizer
{
/// <summary>
/// </summary>
public class On
public partial class On
{

/// <summary>
/// Provides fluent date accessors for January
/// </summary>
Expand Down Expand Up @@ -3101,5 +3103,5 @@ public static DateTime The31st
get { return new DateTime(DateTime.Now.Year, 12, 31); }
}
}
}
}
}