-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Update Result.String implementation to work as declared in interface #583
Conversation
Can we just delete this method? How are people using this method today? |
It's probably used only for debugging purpose, but removal will be breaking change. At least it would require a note in release notes. |
Probably types020.Result also should be updated for consistency: Line 87 in 8fa373b
|
I vote for just removing the function entirely. |
+1 |
aff3fac
to
0939fbe
Compare
Closes containernetworking#581 Signed-off-by: Piotr Skamruk <piotr.skamruk@gmail.com>
Signed-off-by: Piotr Skamruk <piotr.skamruk@gmail.com>
0939fbe
to
704c56d
Compare
Bump. Funcs removed. |
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.
LGTM
It's a breaking change; I guess will trigger 0.8.0 when we release.
String()
ofResult
declared inpkg/types/types.go
should return aResult
representation in JSON, instead of custom formatted string, which
additionally is not in the same form as described in docstring, which also e.g. is missing
Routes
part.This PR changes this behavior to be consistent with behavior documented in the interface.
As
String()
interface does not have any place for errors - if therewould be any - instead of JSON value output will be an empty string.
Closes #581
edit/update:
As discussed in comments - this PR removes funcs which are turning
Result
to string.