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

[Quest API] Add secondstotime(duration) to Perl and Lua. #1281

Merged
2 commits merged into from
Mar 12, 2021
Merged

Conversation

ghost
Copy link

@ghost ghost commented Mar 1, 2021

Converts time in seconds to human readable string.

  • Add quest::secondstotime(duration) to Perl.
  • Add eq.seconds_to_time(duration) to Lua.

@ghost ghost force-pushed the convert_time branch from b97bd2a to 17c2fc8 Compare March 1, 2021 04:40
@ghost ghost force-pushed the convert_time branch from 17c2fc8 to a50a545 Compare March 1, 2021 05:17
- Add quest::converttime(duration) to Perl.
- Add eq.convert_time(duration) to Lua.
@ghost ghost force-pushed the convert_time branch from a50a545 to e6209f6 Compare March 1, 2021 05:17
@Akkadius
Copy link
Member

Akkadius commented Mar 1, 2021

This is great!

One nit: We should change the method name to humanizetime as converttime is arbitrary and could have many meanings

Lots of libraries use humanize or humantime for similar meaning / understanding

https://momentjscom.readthedocs.io/en/latest/moment/08-durations/03-humanize/

https://zetcode.com/php/carbon/

@splose

This comment was marked as abuse.

@splose

This comment was marked as abuse.

@mackal
Copy link
Member

mackal commented Mar 10, 2021

So like... can we merge this or do I have to do it manually?

Personally I'm fine with the current name, but no one has commented on what @Akkadius suggested.

@ghost
Copy link
Author

ghost commented Mar 12, 2021

Just got back home tonight and had a chance to sit down at my computer again.

I personally think that converttime is a easier to remember and easier to search for name than humanizetime.

I've asked a couple people their opinion about the name as well and they all agreed they'd search for time or convert before they'd ever search humanize or something similar to that.

So I believe we should merge the pull request as is.

@ghost ghost changed the title [Quest API] Add converttime(duration) to Perl and Lua. [Quest API] Add secondstotime(duration) to Perl and Lua. Mar 12, 2021
@ghost ghost merged commit cd08c96 into master Mar 12, 2021
@ghost ghost deleted the convert_time branch March 29, 2021 02:13
This pull request was closed.
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.

5 participants