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

Add IgnoreUnsetFields helper to cmpopts #53

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@dsnet
Member

dsnet commented Nov 6, 2017

IgnoreUnsetFields is useful for performing the equivalent of a "partial"
equality, where only fields based on some template struct are compared,
any everything else is ignored.

This is useful for forward-compatible comparison where the user wants
to ensure struct fields added in the future do not get compared.

Add IgnoreUnsetFields helper to cmpopts
IgnoreUnsetFields is useful for performing the equivalent of a "partial"
equality, where only fields based on some template struct are compared,
any everything else is ignored.

This is useful for forward-compatible comparison where the user wants
to ensure struct fields added in the future do not get compared.

@dsnet dsnet requested a review from neild Nov 6, 2017

@jba

This comment has been minimized.

Show comment
Hide comment
@jba

jba Nov 7, 2017

Collaborator

This could be confusing.

IgnoreUnsetFields(MyStruct{A: 1, B:0})

B is ignored even though I "set" it. And that will almost never get caught, because it only increases the number of tests that pass.

At a minimum, rename to IgnoreZeroFields.

Collaborator

jba commented Nov 7, 2017

This could be confusing.

IgnoreUnsetFields(MyStruct{A: 1, B:0})

B is ignored even though I "set" it. And that will almost never get caught, because it only increases the number of tests that pass.

At a minimum, rename to IgnoreZeroFields.

@dsnet

This comment has been minimized.

Show comment
Hide comment
@dsnet

dsnet Dec 8, 2017

Member

I'm revoking this PR. I don't like the fact that I this API forces me to think about what is the non-zero value of a type. A better API would probably be IgnoreFieldsExcept(typ interface{}, names ...string) which is similar to IgnoreFields, but ignores all fields not in the given list. I'm not going to add the new option until I see more cases where it would have been useful.

Member

dsnet commented Dec 8, 2017

I'm revoking this PR. I don't like the fact that I this API forces me to think about what is the non-zero value of a type. A better API would probably be IgnoreFieldsExcept(typ interface{}, names ...string) which is similar to IgnoreFields, but ignores all fields not in the given list. I'm not going to add the new option until I see more cases where it would have been useful.

@dsnet dsnet closed this Dec 8, 2017

@dsnet dsnet deleted the ignore-unset branch Dec 8, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment