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

Support min, mins abbreviations for minutes #14

Merged
merged 2 commits into from
May 4, 2017
Merged

Support min, mins abbreviations for minutes #14

merged 2 commits into from
May 4, 2017

Conversation

brandonio21
Copy link
Contributor

Problem

I've been very happy with the parse_timespan functionality of humanfriendly. The only issue I've had is that sometimes I'd like to say "3 min" or "3 mins" for minutes.

Solution

I've changed time_units to allow for multiple abbreviations per time unit. I also changed parse_timespan to look through all abbreviations when parsing times. Finally, I added "min" and "mins" as abbreviations for minutes.

Outcome

Parsing "3 min" and "3 mins" is now supported!

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.7%) to 94.824% when pulling 9962618 on brandonio21:min into 323e523 on xolox:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.7%) to 94.824% when pulling 6d5fb86 on brandonio21:min into 323e523 on xolox:master.

@brandonio21
Copy link
Contributor Author

Although coverage fails, I believe the new functionality to be sufficiently tested. (y)

xolox added a commit that referenced this pull request May 4, 2017
In this merge commit I'm making two changes compared to #14:

 1. I've documented the supported time units and abbreviations.
 2. I've changed the body of parse_timespan() to remove the three
    list allocations per iteration. Before the pull request there
    was one tuple allocation per iteration. Now there are two
    equality comparisons and a `contains' test (in) per iteration.
    The slightly less readable code seems like a small sacrifice
    in this case. Maybe I've programmed too much C? :-)
xolox added a commit that referenced this pull request May 4, 2017
I've decided to bump the major version number after merging pull request
#14 because the `humanfriendly.time_units' data structure was changed.

Even though this module scope variable isn't included in the online
documentation, nothing stops users from importing it anyway, so this
change is technically backwards incompatible.

Besides, version numbers are cheap. In fact, they are infinite! :-)
@xolox xolox merged commit 6d5fb86 into xolox:master May 4, 2017
@xolox
Copy link
Owner

xolox commented May 4, 2017

Hi Brandon and thanks for the pull request! This is now merged and released as humanfriendly 3.0.

@brandonio21
Copy link
Contributor Author

Bravo! Thank you very much!

@brandonio21 brandonio21 deleted the min branch May 5, 2017 02:29
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.

3 participants