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

New reminder features #466

Open
wants to merge 12 commits into
base: master
from

Conversation

@Akarys42
Copy link
Member

commented Sep 30, 2019

Hi, this PR add :

  • reminder name and timedelta in the confirmation message
  • jump url to the message where the reminder was set in the deliver message

I marked this PR as draft because I don’t have my test environment setup yet so theses changes aren’t tested for now. Now tested

This PR closes #241.

Akarys42 added 3 commits Sep 29, 2019
@sco1 sco1 added this to Needs review in Bot Tracking Oct 1, 2019
Fix some liting error

Correct error

Fix linting (maybe)
@Akarys42 Akarys42 force-pushed the Akarys42:reminder-up branch from aee2e79 to 387bb50 Oct 2, 2019
Akarys42 added 3 commits Oct 6, 2019
@Akarys42 Akarys42 marked this pull request as ready for review Oct 6, 2019
@Akarys42

This comment has been minimized.

Copy link
Member Author

commented Oct 6, 2019

Now tested and ready to go ! The only problem is that the jump_url is never added to the database, gdude said it is apparently because there is no jump_url in the model.

Copy link
Member

left a comment

This PR can't be merged until a PR for site that adds support for storing the jump_url field in the database is merged.

bot/cogs/reminders.py Outdated Show resolved Hide resolved
bot/cogs/reminders.py Show resolved Hide resolved
bot/cogs/reminders.py Outdated Show resolved Hide resolved
Akarys42 and others added 3 commits Oct 7, 2019
Co-Authored-By: Sebastiaan Zeeff <33516116+SebastiaanZ@users.noreply.github.com>
@lemonsaurus

This comment has been minimized.

Copy link
Member

commented Oct 11, 2019

python-discord/site#277 was closed. So this is blocked until that change is made.

@Akarys42 did you at least open an issue on the site repo so that someone else can make this change if you don't intend to make it yourself?

@Akarys42

This comment has been minimized.

Copy link
Member Author

commented Oct 11, 2019

I didn’t opened a issue because I want to do it myself, I’m with my family this weekend, so I will probably do it next week is it okay for you.

@Akarys42

This comment has been minimized.

Copy link
Member Author

commented Oct 19, 2019

This PR add the jump_url field to the reminder model : python-discord/site#288. The bot PR need to be merged first

bot/cogs/reminders.py Outdated Show resolved Hide resolved
@lemonsaurus

This comment has been minimized.

Copy link
Member

commented Oct 20, 2019

This doesn't pass lint. Please address.

Copy link
Member

left a comment

These requested changes depend on the comments I left for python-discord/site#288.

bot/cogs/reminders.py Outdated Show resolved Hide resolved
@Akarys42 Akarys42 force-pushed the Akarys42:reminder-up branch from 3866d08 to 504ff13 Oct 21, 2019
@Akarys42 Akarys42 requested a review from SebastiaanZ Oct 21, 2019
It is now truly backward compatible and use a more DRY method
@Akarys42 Akarys42 force-pushed the Akarys42:reminder-up branch from 504ff13 to 0b02064 Oct 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Bot Tracking
  
Needs review
4 participants
You can’t perform that action at this time.