-
Notifications
You must be signed in to change notification settings - Fork 3
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
fix: localization command names being empty #14
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #14 +/- ##
==========================================
- Coverage 68.13% 67.63% -0.50%
==========================================
Files 7 7
Lines 408 411 +3
Branches 66 66
==========================================
Hits 278 278
- Misses 128 131 +3
Partials 2 2 ☔ View full report in Codecov by Sentry. |
I have a new idea, instead of making a new regexCommand class, the commands.comand method could accept a type parameter, and check at runtime. I think it should be more straight forward |
sorry for the delay on this. |
I have a design question:
I'll update the PR to
And the first point of the two points, if you are good with the 2nd one I can add it too |
Done. Tested in a dummy bot. One thing to note. I didn't add the prefilter of regex to the |
5271a82
to
63bc0fa
Compare
Removed pollution from commit history |
Would you be willing to add automated tests for the two functions that were changed? WDYT? |
Sure, I'll do it asap |
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
thank you so much! ❤️
fixes #13 by removing regExp conversion to, and type support from _lenguages set in Command class:
output quoted in the issue now looks like:
other functionality is not broken