Skip to content

Epoch command#983

Merged
HassanAbouelela merged 37 commits into
python-discord:mainfrom
mathstrains21:epoch-command
Jan 8, 2022
Merged

Epoch command#983
HassanAbouelela merged 37 commits into
python-discord:mainfrom
mathstrains21:epoch-command

Conversation

@b0nes1
Copy link
Copy Markdown
Contributor

@b0nes1 b0nes1 commented Dec 18, 2021

Relevant Issues

Closes #787

Description

Created an "epoch" command which allows you to convert a word date and time to an epoch (Unix timestamp). The parsing for relative dates is done with arrow.dehumanize, absolute dates are parsed with dateutil.parser.

If the entered string is parsed successfully, the epoch is sent to chat along with a drop-down interaction which allows you to format the epoch as one of the discord timestamp styles
image
image

Did you:

Copy link
Copy Markdown
Contributor

@Akarys42 Akarys42 left a comment

Choose a reason for hiding this comment

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

Works pretty well, just a few things

Comment thread bot/exts/utilities/epoch.py Outdated
Comment thread bot/exts/utilities/epoch.py Outdated
Comment thread bot/exts/utilities/epoch.py Outdated
Comment thread bot/exts/utilities/epoch.py
Comment thread bot/exts/utilities/epoch.py Outdated
@b0nes1 b0nes1 marked this pull request as ready for review December 18, 2021 21:26
Sn4u and others added 2 commits December 19, 2021 15:32
@b0nes1 b0nes1 added area: backend Related to internal functionality and utilities category: utilities Related to utilities status: needs review Author is waiting for someone to review and approve type: feature Relating to the functionality of the application. labels Dec 19, 2021
Copy link
Copy Markdown
Contributor

@janine9vn janine9vn left a comment

Choose a reason for hiding this comment

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

Thank you for this PR!

Really solid code, I just have some minor feedback and a small bug with the dropdowns.

Comment thread bot/exts/utilities/epoch.py Outdated
Comment thread bot/exts/utilities/epoch.py Outdated
Comment thread bot/exts/utilities/epoch.py Outdated

**Relative time**
accepted units: "seconds", "minutes", "hours", "days", "weeks", "months", "years"
Eg ".epoch in a month 4 days and 2 hours"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A question more than anything, is it an arrow limitation that .epoch 6 hours and .epoch in 6 hours produce different results? In my ideal world, .epoch 6 hours would mean the same thing as .epoch in 6 hours

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, it seems a relative time can't be parsed without an in .. or ... ago. I guess it's probably best to mention that in the docstring

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this could maybe be manually adjusted, by inserting "in" if it isn't there, and ago is not there

Comment thread bot/exts/utilities/epoch.py Outdated
@b0nes1 b0nes1 requested a review from janine9vn December 22, 2021 16:18
Copy link
Copy Markdown
Contributor

@Shivansh-007 Shivansh-007 left a comment

Choose a reason for hiding this comment

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

Looks good, few minor comments.

Comment thread bot/exts/utilities/epoch.py Outdated
Comment thread bot/exts/utilities/epoch.py Outdated
Comment thread bot/exts/utilities/epoch.py
Comment thread bot/exts/utilities/epoch.py Outdated
Comment thread bot/exts/utilities/epoch.py Outdated
Comment thread bot/exts/utilities/epoch.py Outdated
Comment thread bot/exts/utilities/epoch.py Outdated
Comment thread bot/exts/utilities/epoch.py Outdated
Comment thread bot/exts/utilities/epoch.py Outdated
view = TimestampMenuView(ctx, self._format_dates(date_time), epoch)
original = await ctx.send(f"`{epoch}`", view=view)
if await view.wait(): # wait until expiration before removing the dropdown
await original.edit(view=None)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this can error if the message has somehow been deleted when we try to delete it.

Might not worth be changing, given that there's not a wait_for_deletion or anything, so only a mod can delete it.

Comment thread bot/exts/utilities/epoch.py Outdated
Copy link
Copy Markdown
Contributor

@onerandomusername onerandomusername left a comment

Choose a reason for hiding this comment

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

My actually most important message wasn't saved by github. Nor was my summary.

Looks pretty good, one comment about api requests :)

Comment thread bot/exts/utilities/epoch.py Outdated
Copy link
Copy Markdown
Contributor

@janine9vn janine9vn left a comment

Choose a reason for hiding this comment

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

*chef's kiss* Thank you for implementing this! Your local events lead who is always in need of converting dates to epoch is very appreciative.

I have 2 small comments but they are by no means blockers.

Comment thread bot/exts/utilities/epoch.py Outdated
Comment thread bot/exts/utilities/epoch.py Outdated
Copy link
Copy Markdown
Contributor

@HassanAbouelela HassanAbouelela left a comment

Choose a reason for hiding this comment

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

This is really good. Top-notch stuff, thanks

@HassanAbouelela HassanAbouelela merged commit 97f83b0 into python-discord:main Jan 8, 2022
@mathstrains21 mathstrains21 deleted the epoch-command branch January 9, 2022 09:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: backend Related to internal functionality and utilities category: utilities Related to utilities status: needs review Author is waiting for someone to review and approve type: feature Relating to the functionality of the application.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

.epoch command

8 participants