Ornate Orbits

[janine9vn]

No description provided.

[ThirVondukr]

Project Structure

• Project is neatly structured, but some things could be moved around a bit (e.g. I think that core/ui probably should belong to bot package/module)
• Database operations are encapsulated in repository classes 👍
• Cogs could be used instead of adding handlers directly to bot, but there's no problem with that at that scale

Code Quality

• Transactions are not handled correctly, since multiple transactions could be ran during single operation.
• Would be really great to make WordleGame independent from database, word generator (Really not sure if that's possible here since you don't just use it to generate words) and discord, so it would be a lot easier to test.
• Some logic seems to be out of place, like calculating game status in the repository.

Testing

There's only one test case and I'd really like to see more, it's possible to even run the tests in a DB transaction if that's easier than abstracting or mocking everything.

Other

• Mypy should probably be added to CI too, otherwise there's no guarantee that it's been run on the codebase when you're working in a team.

[ThirVondukr]

Would be great to add Mypy to CI/Github Actions too

[ThirVondukr]

Should probably be logger.info, since syncing commands is a normal startup step

[ThirVondukr]

I'm not sure if I'd subclass bot here, maybe just making a function would have been enough:
(You can also put additional startup code here, e.g. registering cogs)

def create_bot() -> Bot:
   settings = BotSettings()
   bot = Bot(settings.COMMAND_PREFIX, intents=Intents.default())
   
   @bot.event
   async def on_ready(self) -> None:
       pass
   
   return bot

[ThirVondukr]

Instead of making a global Bot instance you can use Cogs instead: https://discordpy.readthedocs.io/en/stable/ext/commands/cogs.html
That way when the bot code base gets larger you would have less problems

[ThirVondukr]

EDIT: Since you're using commands, you probably need CommandTree instead: https://discordpy.readthedocs.io/en/stable/interactions/api.html?highlight=tree#discord.app_commands.CommandTree

[ThirVondukr]

Function name could probably be improved, e.g. show_trivial_question, judging from the docstring 😃

[ThirVondukr]

Again, would be create to have all of these fields as __init__ parameters instead, or as a part of different method signature. Also you can create a separate dataclass that would hold all that configuration to not overload the __init__:

@dataclasses.dataclass
class WordGeneratorConfig:
    corpora_wordnet: str = "corpora/wordnet"
    wordnet: str = "wordnet"

    word_length_min: int = 5
    word_length_max: int = 15
    word_amount_hard: int = 1
    word_amount_easy: int = 5
    

class WordGenerator:
    """The class for generating a random word between 5 to 10 chars."""

    def __init__(self, config: WordGeneratorConfig) -> None: ...

[ThirVondukr]

You should copy and install all the dependencies before copying application code, otherwise changing anything in your ./app directory in that case would cause all the next layers to be invalidated, so your dependencies would have to be installed again.
I think it should look like this:

COPY ./requirements-dev.txt .

RUN pip install watchdog==4.0.1
RUN pip install -r requirements-dev.txt

COPY ./app/ ./app/
COPY ./trivia.db ./trivia.db

[ThirVondukr]

Since you have a pyproject.toml file it would be preferred to use that for declaring your dependencies, there's a lot of tools that work with it, e.g. poetry and pdm

[ThirVondukr]

Setting this to true leads to mypy just ignoring incorrect imports (e.g. if you don't have such a file), so it would be better to set this to False.

[ThirVondukr]

Would be better to use pydantic-settings here