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

Fix difference between two date times with asFloat param #267

Merged
merged 1 commit into from
Mar 26, 2024

Conversation

samip77
Copy link
Contributor

@samip77 samip77 commented Mar 26, 2024

What does this PR do?
The difference between two date times had wrong test case which was hiding the actual issue.

The documentation for diff function mentioned:

  /// The optional [asFloat] parameter determines whether to return the
  /// difference as a floating-point number. The default value is [false].
  ///

but the asFloat default value was true

  num diff(Jiffy jiffy, {Unit unit = Unit.microsecond, bool asFloat = true}) {
    return _display.diff(dateTime, jiffy.dateTime, unit, asFloat);
  }

and in the inner function the logic was wrong

return asFloat ? _asFloor(diff) : diff;

Along with that the test case has wrong assumption, because of which it passed the test.

Issue Reference

Fixes #

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Maintenance change (non-breaking change such as upgrading a dependency, refactoring, or making a lint fix)
  • Documentation update
  • Breaking change (a fix or feature that would cause existing functionality to change)

Screenshots

If applicable, add screenshots to help explain your problem.

Checklist

  • [ x] Wrote additional tests, if needed
  • All tests have passed, you can find the scripts from the ./bin folder
  • [ x] I have updated the documentation accordingly, if needed.

Additional Information

@samip77 samip77 requested a review from jama5262 as a code owner March 26, 2024 07:45
@jama5262
Copy link
Owner

@samip77 Thanks for the contribution, will take a look

@samip77
Copy link
Contributor Author

samip77 commented Mar 26, 2024

@samip77 Thanks for the contribution, will take a look

Thanks, I had missed few test cases. updated them as well.

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.

2 participants