-
-
Notifications
You must be signed in to change notification settings - Fork 47
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
Argument parsing for commands #136
Conversation
Suggestion: What about adding unit tests for args parsing? It would be useful for things like edge cases. |
I wouldn't really know how that test would look like? |
This is now added and I feel like the pr is ready for the final review now |
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!
these were my last changes, imo ready to be landed 🚀 |
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! 🎉 I think rest
would be more useful if it was string instead of string[]
I can fix that for sure. then it's good to land I feel like |
Yes. |
About
Implements the feature request of #134
Status
Not done!
These changes have been tested against Discord API or contain no API change.
This PR introduces some Breaking changes.
Add parser
Intergrate parser into message processing
Add dynamic return type