-
-
Notifications
You must be signed in to change notification settings - Fork 94
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
#480 Automatic Coproduct Rename from/to Protobuf GeneratedEnum #489
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #489 +/- ##
==========================================
+ Coverage 90.06% 90.20% +0.13%
==========================================
Files 125 125
Lines 4763 4759 -4
Branches 430 412 -18
==========================================
+ Hits 4290 4293 +3
+ Misses 473 466 -7 ☔ View full report in Codecov by Sentry. |
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.
Thank you for the effort! I left a comment.
@@ -44,6 +44,16 @@ object TransformedNamesComparison { | |||
def namesMatch(fromName: String, toName: String): Boolean = fromName.equalsIgnoreCase(toName) | |||
} | |||
|
|||
case object GeneratedProtobufEnumEquality extends TransformedNamesComparison { |
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 would either name it CamelCaseUnderscoreCaseEquality
(to e.g. use this with Java enums as well, maybe it should be a default for it? Maybe it should be named JavaEnumAware
?) or keep it in chimneyProtobuf
module. I wouldn't put some integration-specific conventions in the core package.
Hey @saeltz 👋 , do you still want to work on this? |
Hey @MateuszKubuszok, this currently matches too broadly. For this special use case I would probably rather go for something like moia-oss/teleproto#326 by @lbialy. |
I'm not entirely happy with the
TransformedNamesComparison
because it matches too broadly. I'm open for better suggestions.I want to achieve a comparison for the following:
From/To