-
Notifications
You must be signed in to change notification settings - Fork 17
Feature/new events #152
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
Feature/new events #152
Conversation
…on to be HTB account based.
…d a placeholder for future implementation to fetch link state from HTB Account.
1. Rewrote the verification instructions and moved them to their own helper function 2. Replaced both the identify and verify commands to send these new instructions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had the opportunity to review the code, even though the PR is still draft. Good job, some minor improvements. In general for the BaseHandler and the other handlers that implement it, I would consider making some methods private.
| logger = logging.getLogger(__name__) | ||
|
|
||
|
|
||
| BOT_TYPE = TypeVar("BOT_TYPE", "Bot", DiscordBot) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this really needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was an attempt to try to tame mypy but it is not really necessary no.
…ntification code.
…factor AccountHandler for improved property handling
…0 response for unhandled exceptions
dimoschi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
@0xRy4n should we merge and release? |
|
good on my side |
Types of changes
What types of changes does your code introduce?
Put an
xin the boxes that apply.Proposed changes
This does the following:
Checklist
Put an
xin the boxes that apply.doc.
Additional context
Add any other context or screenshots here.