-
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 CustomUnmarshaler implementations to use config.Parser, prove the interface #2810
Conversation
ecc42d9
to
ea50cdd
Compare
Codecov Report
@@ Coverage Diff @@
## main #2810 +/- ##
=======================================
Coverage 91.66% 91.67%
=======================================
Files 293 293
Lines 15629 15633 +4
=======================================
+ Hits 14327 14331 +4
+ Misses 891 890 -1
- Partials 411 412 +1
Continue to review full report at Codecov.
|
ea50cdd
to
a45b4c3
Compare
…the interface Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
a45b4c3
to
241e46c
Compare
@mx-psi change the PR to not modify the CustomUnmarshaler interface yet. Reason is that I think better to just deprecate this completely. |
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.
Looks good, the PR title should be changed if we are not changing the implementations.
What do you want to do instead? |
Start adding this #2597 and just mark the component unmarshaler as deprecated/remove it. If I change the component unmarshaler users will first have to update to that, then remove and update to the config one so there will be 2 changes |
We are, because all implementation are first convert to Parser and work only on the parser interface. So now I am confident that Parser has all the functionality I need |
Right, makes sense, let's keep it as it is then.
Can you have a look at the last comments on #2597 and to PR #2802 then? If you want I can work on #2597 on Monday if we agree on the interface we want (which I am guessing will be similar to the one for validation) |
Fixes #2735
Signed-off-by: Bogdan Drutu bogdandrutu@gmail.com