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

tools.time: improve seconds_to_human #2026

Merged
merged 2 commits into from
Feb 25, 2021
Merged

Conversation

Exirel
Copy link
Contributor

@Exirel Exirel commented Feb 6, 2021

Description

Fix #1824

Expose intermediate functions: one to split a time in seconds into a tuple of (years, months, days, hours, minutes, seconds) and the other that take that information to return a tuple of 2-value tuple with (value, unit).

Also, while I was working on making second_to_human more reusable, I added a granularity parameter, because why not.

Checklist

  • I have read CONTRIBUTING.md
  • I can and do license this contribution under the EFLv2
  • No issues are reported by make qa (runs make quality and make test)
  • I have tested the functionality of the things this change touches

@Exirel Exirel added this to the 7.1.0 milestone Feb 6, 2021
@Exirel Exirel requested a review from dgw February 6, 2021 00:25
Copy link
Member

@dgw dgw left a comment

Choose a reason for hiding this comment

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

Just some nemesiS fighting in here. I'd like for the granularity parameter to round the value up if it's close to the next unit (e.g. 1h20m = "1 hour" but 1h40m = "2 hours"), but I don't even need to wait for you to say "let's do that in a future PR" because I already think that. x) I'll put that idea on the list for 8.0.

sopel/tools/time.py Outdated Show resolved Hide resolved
sopel/tools/time.py Outdated Show resolved Hide resolved
sopel/tools/time.py Outdated Show resolved Hide resolved
sopel/tools/time.py Outdated Show resolved Hide resolved
@Exirel Exirel requested a review from dgw February 7, 2021 14:22
Copy link
Member

@dgw dgw left a comment

Choose a reason for hiding this comment

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

Imagine forgetting to re-review a PR for two weeks. 🤯

@Exirel
Copy link
Contributor Author

Exirel commented Feb 25, 2021

Squashed & rebased.

@dgw dgw merged commit c417d8e into sopel-irc:master Feb 25, 2021
@Exirel Exirel deleted the raw-seconds-to-human branch May 1, 2021 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tools.time feature: "Raw" mode for seconds_to_human()
2 participants