[Feature] add run_monthly() method#1705
[Feature] add run_monthly() method#1705Bibo-Joshi merged 13 commits intopython-telegram-bot:masterfrom
Conversation
Bibo-Joshi
left a comment
There was a problem hiding this comment.
Thanks for the PR @daviddl9. Please see my comments.
Bibo-Joshi
left a comment
There was a problem hiding this comment.
Thanks for the updates!
Besides my comments, we'll also need a tests for day_is_strict. Should've mentioned earlier …
Bibo-Joshi
left a comment
There was a problem hiding this comment.
Thanks for the new update!
I have some minor comments left, but I will have to do another thorough review after them.
|
thanks for your comments! have made the changes you requested |
Bibo-Joshi
left a comment
There was a problem hiding this comment.
Hi. It seems one comment went missing from my last review. Sorry for that, since that's certainly my fault. I added it below.
The failing test is unrelated but we have a fix for than in master now. Could you merge master?
Thanks for putting up with my nitpicking ;)
Bibo-Joshi
left a comment
There was a problem hiding this comment.
Hi! Mostly looks good to me now, thanks for your patience :) I wanted to some final details (remove duplicate use of calendar.monthrange(next_year, next_month)[1] and calendar.monthrange(dt.year, dt.month)[1], but it seems you have disabled the option that allows me to do so. Would you mind checking the corresponding box (should be in the column to the right of the PR conversation at the very bottom)?
Hey Bibo, are you referring to the box that says "allow edit from maintainers"? if so, the box is already checked. If not, please let me know which box you want me to check |
|
Okay, I was just too stupid … |
|
If #1685 is merged before this, the changes from there must be added |
|
Just one quick question: Why is the urrlib3 submodule updated? |
# Conflicts: # telegram/ext/jobqueue.py # tests/test_jobqueue.py
# Conflicts: # tests/test_jobqueue.py
This PR exposes a
run_monthly()method that allows users to schedule monthly jobs. This pr also closes #1697