Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Poller cleanup #12

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

RustyNova016
Copy link
Contributor

I found some more area to clarify.

I refactored the Poller class to add a lot more comments, as well as splitting the unreadable match statement.

For the actual poll, I split both edit data polling and note data polling. This prevent weaving two different flows in the same function, and separate concerns.

I would also put the functions in the looper module into the Poller class directly, but I'm not sure if it is intentionally separated.

@yellowHatpro
Copy link
Collaborator

I found some more area to clarify.

I refactored the Poller class to add a lot more comments, as well as splitting the unreadable match statement.

For the actual poll, I split both edit data polling and note data polling. This prevent weaving two different flows in the same function, and separate concerns.

I would also put the functions in the looper module into the Poller class directly, but I'm not sure if it is intentionally separated.

Regarding looper and poller, its fine for them to coexist. A poll_db method in looper is meant to run on each poll. I only wrote it in the beginning when I was just starting, so feel free to improve the code structure 😁

@yellowHatpro
Copy link
Collaborator

Is it still a draft PR, or you want me to merge it to main?

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.

2 participants