-
-
Notifications
You must be signed in to change notification settings - Fork 208
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: replace roman numeral v2 #3003
Fix: replace roman numeral v2 #3003
Conversation
One or more Detekt Failures were detected:
|
I have detected some issues with your pull request: Body issues: Title issues: Please fix these issues. For the correct format, refer to the pull request template. |
One or more Detekt Failures were detected:
|
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.
Please make some unit tests for roman numerals so that it is easier to test if it is working as intended
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.
as my patch in dms: i dont think using regexes is appropriate here, as well as caching is missing. lets discuss
I have detected some issues with your pull request: Body issues: Please fix these issues. For the correct format, refer to the pull request template. |
One or more Detekt Failures were detected:
|
What
Fixed the ReplaceRomanNumeral Feature replacing things it shouldn't. Small changes may follow.
Changelog Fixes