Apply suggestions from code review

Co-Authored-By: Mark <[email protected]>

17 files changed, 39 insertions, 29 deletions

diff --git a/bot/api.py b/bot/api.py
index 5ab554052..3fca5db57 100644
--- a/bot/api.py
+++ b/bot/api.py
@@ -11,7 +11,7 @@ log = logging.getLogger(__name__)
 
 
 class ResponseCodeError(ValueError):
-    """Exception representing a non-OK response code."""
+    """Raised when a non-OK HTTP response is received."""
 
     def __init__(
         self,
@@ -97,10 +97,8 @@ def loop_is_running() -> bool:
     Determine if there is a running asyncio event loop.
 
     This helps enable "call this when event loop is running" logic (see: Twisted's `callWhenRunning`),
-    which is currently not provided by asyncio
+    which is currently not provided by asyncio.
     """
-    # asyncio does not have a way to say "call this when the event
-    # loop is running", see e.g. `callWhenRunning` from twisted.
 
     try:
         asyncio.get_running_loop()
@@ -140,10 +138,13 @@ class APILoggingHandler(logging.StreamHandler):
     def emit(self, record: logging.LogRecord) -> None:
         """
         Determine if a log record should be shipped to the logging API.
+        
+        If the asyncio event loop is not yet running, log records will instead be put in a queue
+        which will be consumed once the event loop is running.
 
         The following two conditions are set:
-            1. Do not log anything below DEBUG
-            2. Ignore log records from the logging handler
+            1. Do not log anything below DEBUG (only applies to the monkeypatched `TRACE` level)
+            2. Ignore log records originating from this logging handler itself to prevent infinite recursion
         """
         # Two checks are performed here:
         if (
@@ -176,7 +177,7 @@ class APILoggingHandler(logging.StreamHandler):
                 self.schedule_queued_tasks()
 
     def schedule_queued_tasks(self) -> None:
-        """Logging task scheduler."""
+        """Consume the queue and schedule the logging of each queued record."""
         for task in self.queue:
             asyncio.create_task(task)
 
diff --git a/bot/cogs/bot.py b/bot/cogs/bot.py
index 7be04693c..324d2ccd3 100644
--- a/bot/cogs/bot.py
+++ b/bot/cogs/bot.py
@@ -82,7 +82,7 @@ class Bot(Cog):
         embed = Embed(description=text)
         await ctx.send(embed=embed)
 
-    def codeblock_stripping(self, msg: str, bad_ticks: bool) -> Optional[Tuple[Tuple[str, Optional[str]], str]]:
+    def codeblock_stripping(self, msg: str, bad_ticks: bool) -> Optional[Tuple[Tuple[str, ...], str]]:
         """
         Strip msg in order to find Python code.
 
@@ -196,7 +196,7 @@ class Bot(Cog):
 
         Tries to strip out REPL Python code out of msg and returns the stripped msg.
 
-        Returns a second boolean output if REPL code was found in the input msg.
+        Returns True for the boolean if REPL code was found in the input msg.
         """
         final = ""
         for line in msg.splitlines(keepends=True):
diff --git a/bot/cogs/clean.py b/bot/cogs/clean.py
index da1ae8b9b..1c0c9a7a8 100644
--- a/bot/cogs/clean.py
+++ b/bot/cogs/clean.py
@@ -43,7 +43,7 @@ class Clean(Cog):
     ) -> None:
         """A helper function that does the actual message cleaning."""
         def predicate_bots_only(message: Message) -> bool:
-            """Returns true if the message was sent by a bot."""
+            """Return True if the message was sent by a bot."""
             return message.author.bot
 
         def predicate_specific_user(message: Message) -> bool:
diff --git a/bot/cogs/defcon.py b/bot/cogs/defcon.py
index 9d530af64..936147c8f 100644
--- a/bot/cogs/defcon.py
+++ b/bot/cogs/defcon.py
@@ -69,7 +69,7 @@ class Defcon(Cog):
 
     @Cog.listener()
     async def on_member_join(self, member: Member) -> None:
-        """If DEFON is enabled, check newly joining users to see if they meet the account age threshold."""
+        """If DEFCON is enabled, check newly joining users to see if they meet the account age threshold."""
         if self.enabled and self.days.days > 0:
             now = datetime.utcnow()
 
diff --git a/bot/cogs/filtering.py b/bot/cogs/filtering.py
index 2eb53e61b..9cd1b7203 100644
--- a/bot/cogs/filtering.py
+++ b/bot/cogs/filtering.py
@@ -104,7 +104,7 @@ class Filtering(Cog):
         """
         Invoke message filter for message edits.
 
-        If there have been multiple edits, calculate the time delta from the previous edit
+        If there have been multiple edits, calculate the time delta from the previous edit.
         """
         if not before.edited_at:
             delta = relativedelta(after.edited_at, before.created_at).microseconds
diff --git a/bot/cogs/modlog.py b/bot/cogs/modlog.py
index 15c293d92..68424d268 100644
--- a/bot/cogs/modlog.py
+++ b/bot/cogs/modlog.py
@@ -67,7 +67,7 @@ class ModLog(Cog, name="ModLog"):
         return f"{URLs.site_logs_view}/{response['id']}"
 
     def ignore(self, event: Event, *items: int) -> None:
-        """Add event to ignored events to suppress log emitting."""
+        """Add event to ignored events to suppress log emission."""
         for item in items:
             if item not in self._ignored[event]:
                 self._ignored[event].append(item)
diff --git a/bot/cogs/off_topic_names.py b/bot/cogs/off_topic_names.py
index 53e693b89..5a59dc663 100644
--- a/bot/cogs/off_topic_names.py
+++ b/bot/cogs/off_topic_names.py
@@ -20,7 +20,7 @@ class OffTopicName(Converter):
 
     @staticmethod
     async def convert(ctx: Context, argument: str) -> str:
-        """Attempt to replace any invalid characters with their approximate unicode equivalent."""
+        """Attempt to replace any invalid characters with their approximate Unicode equivalent."""
         allowed_characters = "ABCDEFGHIJKLMNOPQRSTUVWXYZ!?'`-"
 
         if not (2 <= len(argument) <= 96):
diff --git a/bot/cogs/reminders.py b/bot/cogs/reminders.py
index a96684db3..8460de91f 100644
--- a/bot/cogs/reminders.py
+++ b/bot/cogs/reminders.py
@@ -32,7 +32,7 @@ class Reminders(Scheduler, Cog):
 
     @Cog.listener()
     async def on_ready(self) -> None:
-        """Reschedule all current reminders."""
+        """Get all current reminders from the API and reschedule them."""
         response = await self.bot.api_client.get(
             'bot/reminders',
             params={'active': 'true'}
@@ -77,7 +77,7 @@ class Reminders(Scheduler, Cog):
         self.cancel_task(reminder_id)
 
     async def _delete_reminder(self, reminder_id: str) -> None:
-        """Delete a reminder from the database, given its ID, and cancels the running task."""
+        """Delete a reminder from the database, given its ID, and cancel the running task."""
         await self.bot.api_client.delete('bot/reminders/' + str(reminder_id))
 
         # Now we can remove it from the schedule list
@@ -239,7 +239,7 @@ class Reminders(Scheduler, Cog):
     @edit_reminder_group.command(name="duration", aliases=("time",))
     async def edit_reminder_duration(self, ctx: Context, id_: int, expiration: ExpirationDate) -> None:
         """
-        Edit one of your reminders' expiration.
+         Edit one of your reminder's expiration.
 
         Expiration is parsed per: http://strftime.org/
         """
@@ -258,7 +258,7 @@ class Reminders(Scheduler, Cog):
 
     @edit_reminder_group.command(name="content", aliases=("reason",))
     async def edit_reminder_content(self, ctx: Context, id_: int, *, content: str) -> None:
-        """Edit one of your reminders' content."""
+        """Edit one of your reminder's content."""
         # Send the request to update the reminder in the database
         reminder = await self.bot.api_client.patch(
             'bot/reminders/' + str(id_),
diff --git a/bot/cogs/security.py b/bot/cogs/security.py
index 4960aa896..316b33d6b 100644
--- a/bot/cogs/security.py
+++ b/bot/cogs/security.py
@@ -14,11 +14,11 @@ class Security(Cog):
         self.bot.check(self.check_on_guild)  # Global commands check - commands can't be run in a DM
 
     def check_not_bot(self, ctx: Context) -> bool:
-        """Check if Context instance author is not a bot."""
+        """Check if the context is a bot user."""
         return not ctx.author.bot
 
     def check_on_guild(self, ctx: Context) -> bool:
-        """Check if Context instance has a guild attribute."""
+        """Check if the context is in a guild."""
         if ctx.guild is None:
             raise NoPrivateMessage("This command cannot be used in private messages.")
         return True
diff --git a/bot/cogs/site.py b/bot/cogs/site.py
index aadc9c632..4a423faa9 100644
--- a/bot/cogs/site.py
+++ b/bot/cogs/site.py
@@ -95,7 +95,7 @@ class Site(Cog):
     @site_group.command(aliases=['r', 'rule'], name='rules')
     @redirect_output(destination_channel=Channels.bot, bypass_roles=STAFF_ROLES)
     async def site_rules(self, ctx: Context, *rules: int) -> None:
-        """Provides a link to the `rules` endpoint of the website, or displays specific rule(s), if requested."""
+        """Provides a link to all rules or, if specified, displays specific rule(s)."""
         rules_embed = Embed(title='Rules', color=Colour.blurple())
         rules_embed.url = f"{PAGES_URL}/rules"
 
diff --git a/bot/cogs/sync/syncers.py b/bot/cogs/sync/syncers.py
index 689d3736e..2cc5a66e1 100644
--- a/bot/cogs/sync/syncers.py
+++ b/bot/cogs/sync/syncers.py
@@ -166,7 +166,7 @@ def get_users_for_sync(
 
 async def sync_users(bot: Bot, guild: Guild) -> Tuple[int, int, None]:
     """
-    Synchronize users found on the given `guild` with the ones on the API.
+    Synchronize users found in the given `guild` with the ones in the API.
 
     Arguments:
         bot (discord.ext.commands.Bot):
diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py
index a437992ee..0c4819f66 100644
--- a/bot/cogs/verification.py
+++ b/bot/cogs/verification.py
@@ -148,7 +148,6 @@ class Verification(Cog):
     async def cog_command_error(ctx: Context, error: Exception) -> None:
         """Check for & ignore any InChannelCheckFailure."""
         if isinstance(error, InChannelCheckFailure):
-            # Do nothing; just ignore this error
             error.handled = True
 
     @staticmethod
diff --git a/bot/cogs/watchchannels/watchchannel.py b/bot/cogs/watchchannels/watchchannel.py
index 1a3f5b18c..3af97e7fe 100644
--- a/bot/cogs/watchchannels/watchchannel.py
+++ b/bot/cogs/watchchannels/watchchannel.py
@@ -42,7 +42,7 @@ def proxy_user(user_id: str) -> Object:
 
 @dataclass
 class MessageHistory:
-    """Represent the watch channel's message history."""
+    """Represents a watch channel's message history."""
 
     last_author: Optional[int] = None
     last_channel: Optional[int] = None
diff --git a/bot/decorators.py b/bot/decorators.py
index 9a14d8df4..33a6bcadd 100644
--- a/bot/decorators.py
+++ b/bot/decorators.py
@@ -18,7 +18,7 @@ log = logging.getLogger(__name__)
 
 
 class InChannelCheckFailure(CheckFailure):
-    """In channel check failure exception."""
+    """Raised when a check fails for a message being sent in a whitelisted channel."""
 
     def __init__(self, *channels: int):
         self.channels = channels
diff --git a/bot/pagination.py b/bot/pagination.py
index 473158b3f..32e289e6a 100644
--- a/bot/pagination.py
+++ b/bot/pagination.py
@@ -18,7 +18,7 @@ log = logging.getLogger(__name__)
 
 
 class EmptyPaginatorEmbed(Exception):
-    """Empty paginator embed exception."""
+    """Raised when attempting to paginate with empty contents."""
 
     pass
 
diff --git a/bot/utils/scheduling.py b/bot/utils/scheduling.py
index 3dec42480..08abd91d7 100644
--- a/bot/utils/scheduling.py
+++ b/bot/utils/scheduling.py
@@ -65,6 +65,6 @@ def create_task(loop: asyncio.AbstractEventLoop, coro_or_future: Union[Coroutine
 
 
 def _silent_exception(future: asyncio.Future) -> None:
-    """Suppress future exception."""
+    """Suppress future's exception."""
     with contextlib.suppress(Exception):
         future.exception()
diff --git a/bot/utils/time.py b/bot/utils/time.py
index fe1c4e3ee..fe3ccc271 100644
--- a/bot/utils/time.py
+++ b/bot/utils/time.py
@@ -26,7 +26,12 @@ def _stringify_time_unit(value: int, unit: str) -> str:
 
 
 def humanize_delta(delta: relativedelta, precision: str = "seconds", max_units: int = 6) -> str:
-    """Returns a human-readable version of the relativedelta."""
+    """
+    Returns a human-readable version of the relativedelta.
+    
+    precision specifies the smallest unit of time to include (e.g. "seconds", "minutes").
+    max_units specifies the maximum number of units of time to include (e.g. 1 may include days but not hours).
+    """
     units = (
         ("years", delta.years),
         ("months", delta.months),
@@ -62,7 +67,12 @@ def humanize_delta(delta: relativedelta, precision: str = "seconds", max_units:
 
 
 def time_since(past_datetime: datetime.datetime, precision: str = "seconds", max_units: int = 6) -> str:
-    """Takes a datetime and returns a human-readable string that describes how long ago that datetime was."""
+    """
+    Takes a datetime and returns a human-readable string that describes how long ago that datetime was.
+    
+    precision specifies the smallest unit of time to include (e.g. "seconds", "minutes").
+    max_units specifies the maximum number of units of time to include (e.g. 1 may include days but not hours).
+    """
     now = datetime.datetime.utcnow()
     delta = abs(relativedelta(now, past_datetime))