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

LANG-1530 another method like formatDurationWords with #506

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

Conversation

mikolajmitura
Copy link

added new method 'formatDurationWordsWithMs' for format duration:
0 days 0 hours 0 minutes 0 seconds 0 milliseconds
still exists old method 'formatDurationWords' which format as below:
0 days 0 hours 0 minutes 0 seconds

@coveralls
Copy link

coveralls commented Mar 29, 2020

Coverage Status

Coverage increased (+0.007%) to 95.101% when pulling de7e11f on mikolajmitura:master into 8c1153d on apache:master.

@garydgregory
Copy link
Member

Hi All,

Before we make this class heavier, let's step back and consider what Java 8's own Duration and Period classes have to offer and how this class should interact/interface with these.

@kinow
Copy link
Member

kinow commented Mar 30, 2020

Hi All,

Before we make this class heavier, let's step back and consider what Java 8's own Duration and Period classes have to offer and how this class should interact/interface with these.

+1

I had a quick look at the code, but didn't want to prepare a review before checking if we run the risk of needing one more test to display time zone with micro seconds, and yet more methods for other things.

Good idea on looking at the SDK classes first.

Copy link
Member

@kinow kinow left a comment

Choose a reason for hiding this comment

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

@mikolajmitura whenever you have time, take a look at @garydgregory 's last comment about SDK classes 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants