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

Diff nil and empty for slice and map #42

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

Conversation

motomux
Copy link

@motomux motomux commented Dec 9, 2016

#41

@cespare
Copy link
Contributor

cespare commented Dec 20, 2016

This would be a nice improvement. If you're using pretty.Diff as part of a test failure message, it can be kind of mysterious why the diff is empty even though reflect.DeepEqual returns false.

@kr any reason not to merge? The change LGTM.

@jaytaylor
Copy link

+1

I'd also like to see this merged!

mkabilov pushed a commit to zalando/postgres-operator that referenced this pull request May 12, 2017
- Use the branch of pretty with this feature fixed:
  kr/pretty#42
- Add the Limit to the resources declaration to avoid dummy
  differences between statefulsets (where both Resource structures
  are empty, but in one case the fields are not mentioned, while
  in another they are assigned to empty values).
@tbg
Copy link

tbg commented Jul 9, 2019

For anyone else wanting this, your best bet is probably to just switch to @motomux' fork at https://github.com/motomux/pretty.

YuriSalesquw pushed a commit to YuriSalesquw/postgres-operator that referenced this pull request Dec 26, 2022
- Use the branch of pretty with this feature fixed:
  kr/pretty#42
- Add the Limit to the resources declaration to avoid dummy
  differences between statefulsets (where both Resource structures
  are empty, but in one case the fields are not mentioned, while
  in another they are assigned to empty values).
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.

None yet

4 participants