Skip to content

Active development - #6

Open
BowGaming wants to merge 10 commits into
mainfrom
Active-development
Open

Active development#6
BowGaming wants to merge 10 commits into
mainfrom
Active-development

Conversation

@BowGaming

Copy link
Copy Markdown
Owner

Fixed all the changes proposed by Rocked, excluding db related stuff

@BowGaming
BowGaming requested a review from Rocked03 July 30, 2026 15:32
Comment thread __pycache__/config.cpython-313.pyc Outdated
Comment thread cogs/review.py Outdated
Comment thread README.md
Comment on lines 40 to 44

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BowGaming BowGaming Jul 31, 2026

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have absolutely no clue what the requirements would be, but I will at this to the to-do list to get this done at a later time

@Rocked03 Rocked03 Jul 31, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, read the article

You list the required packages in the requirements.txt and then you do pip install -r requirements.txt to install them all

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but I don't know what the required packages are (I forgot, I have a horrible memory), so I'll look it up later

Comment thread README.md

@Rocked03 Rocked03 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

have reviewed but have left the most detailed DB-related comments for another PR

Comment thread cogs/review.py Outdated
Comment thread forward_reviews.db Outdated
Comment thread cogs/review.py
Comment thread cogs/review.py Outdated
Comment thread cogs/review.py Outdated
Comment thread cogs/threads.py Outdated
Comment thread cogs/threads.py Outdated
Comment thread cogs/threads.py
return

# Retrieve data from db for following executions
self.cursor.execute(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should be using async everywhere, because synchronous operations will block the execution thread

ie whenever a synchronous SQL query is being executed, then any messages being sent through might be missed because the thread is being kept busy by the sync operation

whereas async lets things run in parallel, it's especially important for what could otherwise block the execution thread for seconds or more, but good practice regardless

https://aiosqlite.omnilib.dev/en/stable/

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good, I will do this later since this a bit of a bigger change. I'll commit the other changes now

Comment thread config.py
Comment on lines +9 to +10
reviews_db = "forward_reviews.db"
threads_db = "forward_threads.db"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are these two separate files?

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those very well could be merged, yes. Maybe a good thing to go over together when we go over all of the db stuff so we do it all right in one go :)

Comment thread main.py Outdated
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