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
Move object diff and field path from apimachinery #82
Move object diff and field path from apimachinery #82
Conversation
38e5c32
to
8babdb0
Compare
} | ||
|
||
// ObjectReflectDiff returns a diff computed through reflection, without serializing to JSON. | ||
func ObjectReflectDiff(a, b interface{}) string { |
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.
@smarterclayton will be happy to have this more available
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.
So am I! This fixes my request in #75.
both make sense to me for use outside k/k |
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.
There are other diff libs out there that produce better output that ours. Should we consider them instead?
go-spew is good, but I think litter
produces better output - should we look at that,
"detect nil map": { | ||
a: map[string]int(nil), | ||
b: map[string]int{}, | ||
out: ` |
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.
this is a great place to use dedent and not have wacky indenting in string literals.
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.
@sttts do you want to switch to dedent in this PR or a follow up?
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.
Would prefer a follow-up. This is a straight move.
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.
The fact that the diff APIs expose their implementation details in their names means we can probably never change these once we publish them. Are we cool with that?
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sttts, thockin The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
I have my doubts those details should be hidden. It makes a big difference how you extract a diff from objects. JSON is different from reflect. |
Perhaps the names should be more meaningfu, rather than
implementation-centric? Or perhaps we should find one-true format that we
can use everywhere.
…On Wed, Feb 13, 2019 at 3:06 AM Dr. Stefan Schimanski < ***@***.***> wrote:
The fact that the diff APIs expose their implementation details in their
names means we can probably never change these once we publish them. Are we
cool with that?
I have my doubts those details should be hidden. It makes a big difference
how you extract a diff from objects. JSON is different from reflect.
—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
<#82 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AFVgVJl-JCa8TiwjaQnYiBAUieybpbbcks5vM_HIgaJpZM4anGNN>
.
|
Both packages are not apimachinery specific, but totally generic (and e.g. useful in k/kube-openapi).