Add commands to turn automatic review posting on or off#1705
Conversation
wookie184
added
a: recruitment
Added on/off as aliases
MarkKoz
left a comment
MarkKoz
left a comment
There was a problem hiding this comment.
The commands don't check the current status, causing them to sometimes perform redundant operations. For example, if autoreview is already enabled, then an enable command will still cause all reviews to be rescheduled. What do you think about adding a check to avoid that?
Apart from that, this would be ready for approval from me. My tests have shown everything is working.
Bluenix2
left a comment
Bluenix2
left a comment
There was a problem hiding this comment.
Should we have punctuation? As in, ":x: Autoreview is already disabled." (currently missing).
| """Show whether automatic posting of reviews is enabled or disabled.""" | ||
| if await self.autoreview_enabled(): | ||
| await ctx.send("Autoreview is currently enabled") | ||
| else: | ||
| await ctx.send("Autoreview is currently disabled") |
There was a problem hiding this comment.
| """Show whether automatic posting of reviews is enabled or disabled.""" | |
| if await self.autoreview_enabled(): | |
| await ctx.send("Autoreview is currently enabled") | |
| else: | |
| await ctx.send("Autoreview is currently disabled") | |
| """Show whether automatic posting of reviews is enabled or disabled.""" | |
| status = "enabled" if await self.autoreview_enabled() else "disabled" | |
| await ctx.send(f"Autoreview is currently {status}") |
There was a problem hiding this comment.
Neither seems preferable over the other to me. Your suggestion looks neater but the one liner is maybe slightly less readable.
The rest of the cog doesn't seem to include full stops, and I don't think it matters too much anyway, so I don't think so, no. |
4 participants
We may want to temporarily stop new nomination reviews from being automatically posted, so this PR creates three new commands,
tp autoreview on,tp autoreview off, andtp autoreview status, all with fairly self-explanatory names, I think :)This uses RedisCache to store the status so it can be persisted over restarts.