Skip to content
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

Implement EqualIgnoreOrder #285

Closed
wants to merge 3 commits into from
Closed

Implement EqualIgnoreOrder #285

wants to merge 3 commits into from

Conversation

swelly127
Copy link

@swelly127 swelly127 commented Apr 20, 2017

Hey I work at Dropbox, I think this function would be helpful for a lot of people who need to test equalness in proto objects when they don't care about order of elements in slices.

@awalterschulze
Copy link
Member

The first problem here is that gogoprotobuf does not fully support proto.Equal
#13
It has not been tested enough and I am sure there are edge cases, which don't work.
I recommend generating an Equal method using gogoprotobuf's extensions or you are welcome to help me fix proto.Equal by adding more tests.

The second problem is that this code is part of the fork with golang/protobuf. I don't suspect they will go for such a pull request, which means I will have to support this diff and with every merge from golang/protobuf. That means that I will want this feature to be something that a lot of people want.

Maybe a simpler way to solve this is to write a function that uses reflect and recursively sorts all fields that are slices and then does a reflect.DeepEqual after that?
Or if this is only for a specific case, maybe you don't even need to use reflect.

@awalterschulze
Copy link
Member

I am closing this, but just comment if you would like it reopened.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants