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

impl EqualValues 2 objects have (T) Equal(T) bool #1093

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rubensayshi
Copy link
Contributor

Summary

when using EqualValues for 2 objects which are of the same type and implement an (T) Equal(T) bool method, use the method to compare.

Tbh I'd almost put this in Equal, but EqualValues is more correct...

I chose not to implement supporting EqualValues(Foo{1}, &Foo{1}), though I do have a branch with it implemented ...

Motivation

This seems very much inline with many many other languages and makes it much easier to compare custom types, ranging from application level types to decimal.Decimal.

@rubensayshi rubensayshi force-pushed the isequaller-reflection branch from 75f7699 to 7294ac7 Compare June 28, 2021 13:13
@rubensayshi rubensayshi force-pushed the isequaller-reflection branch from 7294ac7 to 215beb6 Compare January 22, 2022 15:19
@arvenil
Copy link

arvenil commented Jun 2, 2023

We had to switch to https://github.com/google/go-cmp for the same reason.

@rowland66
Copy link

We had to switch to https://github.com/google/go-cmp for the same reason.

Do you mean that you integrated google's go-cmp with stretchr, or that you used go-cmp instead. I don't think that go-cmp is a replacement for stretchr.

@dolmen dolmen added enhancement Breaking Change pkg-assert Change related to package testify/assert labels Jul 16, 2023
@dolmen dolmen self-assigned this Jul 16, 2023
…mplement an `(T) Equal(T) bool` method, use the method to compare.
@rubensayshi rubensayshi force-pushed the isequaller-reflection branch from 215beb6 to bb8e80e Compare February 18, 2025 16:40
@rubensayshi
Copy link
Contributor Author

rebased this old MR, I still think this is a good change! we've been using a fork for years now to have this change...

we use decimal.Decimal extensively and DeepEqual doesn't work well for those because the same decimal can be represented in different ways...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking Change enhancement pkg-assert Change related to package testify/assert
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants