-
-
Notifications
You must be signed in to change notification settings - Fork 65
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
Add unitCount
option
#29
Conversation
test.js
Outdated
t.is(m(1000 * 60 * 67, {unitsToShow: 2}), '~1h 7m'); | ||
t.is(m(1000 * 60 * 67 * 24 * 465, {unitsToShow: 1}), '~1y'); | ||
t.is(m(1000 * 60 * 67 * 24 * 465, {unitsToShow: 2}), '~1y 154d'); | ||
t.is(m(1000 * 60 * 67 * 24 * 465, {unitsToShow: 3}), '~1y 154d 6h'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to see some tests with this in combination with the verbose
and compact
option.
readme.md
Outdated
@@ -84,6 +84,13 @@ Default: `false` | |||
|
|||
Only show the first unit: `1h 10m` → `~1h`. | |||
|
|||
##### unitsToShow |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would call the option unitCount
.
You have marked #29 (comment) as resolved, but I don't see it being done. |
unitsToShow
option to limit number of units displayed
unitsToShow
option to limit number of units displayedunitCount
option
You also need to fix the merge conflict. |
Currently, it is only possible to limit the number of units displayed to one unit. This would allow limiting to an arbitrary number of units without breaking the module's existing behavior.