Skip to content

Fix off-by-one error in MAXIMUM_REMINDERS check#3497

Open
oliveman-au wants to merge 2 commits intopython-discord:mainfrom
oliveman-au:fix/reminder-maximum-reminders-off-by-one
Open

Fix off-by-one error in MAXIMUM_REMINDERS check#3497
oliveman-au wants to merge 2 commits intopython-discord:mainfrom
oliveman-au:fix/reminder-maximum-reminders-off-by-one

Conversation

@oliveman-au
Copy link
Copy Markdown

MAXIMUM_REMINDERS is set to 5 and the delete command docstring confirms the limit is 5, but the check uses > instead of >=, meaning users can actually create 6 reminders before being blocked.

Changed > MAXIMUM_REMINDERS to >= MAXIMUM_REMINDERS so the limit is enforced correctly.

@L3viathan
Copy link
Copy Markdown
Contributor

The off-by-one error fix seems fine, but the other hunk seems unrelated (and parts of it are already in another PR).

@oliveman-au
Copy link
Copy Markdown
Author

The off-by-one error fix seems fine, but the other hunk seems unrelated (and parts of it are already in another PR).

reproduced this locally - you can see 6 reminders were created before the bot blocked a 7th. page 1 shows reminders 8326-8328 and page 2 shows 8329-8331, all created before hitting the limit. the 7th attempt got blocked when it should have been the 6th.
Screenshot 2026-04-25 210537
Screenshot 2026-04-25 210606

@oliveman-au
Copy link
Copy Markdown
Author

The off-by-one error fix seems fine, but the other hunk seems unrelated (and parts of it are already in another PR).

yeah sorry about that, the branch was accidentally built on top of the jump_url fix. the only intentional change here is the >= fix, the other hunk is already covered in #3496

@lemonsaurus
Copy link
Copy Markdown
Member

@oliveman-au This PR should be rebased on main so it only contains exactly the fix you're describing, and not this unrelated chunk.

Make sure that all your branches are always based on main, not on other branches you are working on. Every PR should contain an atomic change, not be mixed with other changes.

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.

3 participants