Skip to content

fix: hanukkah command respects month boundaries#959

Merged
jb3 merged 1 commit into
python-discord:mainfrom
onerandomusername:fix-hanukkah-command
Dec 2, 2021
Merged

fix: hanukkah command respects month boundaries#959
jb3 merged 1 commit into
python-discord:mainfrom
onerandomusername:fix-hanukkah-command

Conversation

@onerandomusername
Copy link
Copy Markdown
Contributor

@onerandomusername onerandomusername commented Dec 1, 2021

Relevant Issues

https://canary.discord.com/channels/267624335836053506/635950537262759947/915621871960092712

Description

rewrote hanukkah to use datetime.strptime
left a helper method and some variables in order to allow extending
to use a cache in the future, rather than requesting the api every invoke

that is out of scope of this commit and pull, since the command is
currently broken.

I've only kept the same functionality, without trying to rewrite the
entire command.

Did you:

rewrote hanukkah to use datetime.strptime
left a helper method and some variables in order to allow extending
to use a cache in the future, rather than requesting the api every invoke

that is out of scope of this commit and pull, since the command is
currently broken.

I've only kept the same functionality, without trying to rewrite the
entire command.
@onerandomusername
Copy link
Copy Markdown
Contributor Author

New results:

image

Current result, of which this pr is to fix:

image

Copy link
Copy Markdown
Member

@ChrisLovering ChrisLovering left a comment

Choose a reason for hiding this comment

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

Works well, manually tested by editing the date it was using for today.

Copy link
Copy Markdown
Contributor

@Shom770 Shom770 left a comment

Choose a reason for hiding this comment

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

LGTM

@jb3 jb3 merged commit bdcea85 into python-discord:main Dec 2, 2021
@onerandomusername onerandomusername deleted the fix-hanukkah-command branch December 2, 2021 00:08
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.

4 participants