-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Change With*Unmarshallers signatures #2973
Change With*Unmarshallers signatures #2973
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2973 +/- ##
=======================================
Coverage 91.63% 91.63%
=======================================
Files 312 312
Lines 15404 15404
=======================================
Hits 14116 14116
Misses 881 881
Partials 407 407
Continue to review full report at Codecov.
|
The CI failure seems unrelated to my change. Could any try to re-run the CI? |
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.
We use 'Unmarshaler' (single 'l') on other places in the Collector while we use 'Unmarshallers' here. Both forms are valid English spelling but I think we should consider using a consistent spelling. Maybe this is a good PR to do so (if we want to use the single 'l' version at least)?
This sounds good. I rollbacked my changes to the variables. I'd like to have another PR to rename (Also, please feel free to rollback my last commit if you choose the double |
@sincejune @mx-psi good catch, I would prefer the one "l" version (less spelling). |
Description: