-
Notifications
You must be signed in to change notification settings - Fork 107
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
Added UnmarshalOpt to allow a best effort unmarshal #863
Conversation
…hal still will not pass, but now the unmarshal will continue, logging any errors it encounters instead of exiting immediately.
… into best_effort_unmarshal
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.
PTAL at the presubmit failures.
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.
Thanks!
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.
Missed reviewing the coverage, can you add coverage for one part of the code and the rest looks good.
…ror string when BestEffortUnmarshal is passed.
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, LGTM
Sorry, thought I had run gofmt, but forgot to. |
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.
Thanks! merged
Added an option to allow a failing unmarshal will now pass - instead of failing, it will simply log the errors and continue the unmarshaling process. This is so that we can generate diffs for the entirety of the SetRequest, even if there's a compliance error.