-
Notifications
You must be signed in to change notification settings - Fork 842
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
CommonClient: add more docstrings and comments #3821
Conversation
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 is awesome, thank you 🙏
The Command Processor will parse every method of the class that starts with "_cmd_" as a command to be called | ||
when parsing user input, i.e. _cmd_exit will be called when the user sends the command "/exit". | ||
|
||
The decorator @mark_raw can be imported from MultiServer and tells the parser to only split on the first |
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 is super helpful, I didn't know about mark_raw
🔥
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.
Cannot speak for the correctness of the new docstrings as I am not very familiar with CommonClient code. However, taking a brief look at the code that it documents, nothing seems out of place.
Giving a soft approval as long as someone with more experience with client code looks at this.
What is this fixing or adding?
adds more documentation to what CommonClient is doing and how you are expected to extend it
all descriptions are based on my understanding of the relevant code so correct me if i'm wrong / if there is better wording
How was this tested?
reading
If this makes graphical changes, please attach screenshots.