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

Add config options AlwaysIncludeTrailingComma, DisableNilValues #60

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

atombender
Copy link
Contributor

Adds config option AlwaysIncludeTrailingComma, which forces the writing of commas in arrays/slices/maps. Useful in tests to avoid false positives when diffing. This is a prerequisite to resolving this Testify issue.

Building on that commit is a new config option DisableNilValues. Background: We have a bunch of tests that use Spew to generate "golden" test data. This is great, but often one adds a new, optional field to a struct that's part of this data, and suddenly all the test data is broken because the new filed is nil everywhere. It's onerous to update all test data for such zero-value fields. This fixes that.

…g of commas

in arrays/slices/maps. Useful in tests to avoid false positives when diffing.
…o include

nil fields. This is useful in tests that rely on "golden" test data: Often one
adds a new, optional field to a struct, and it's onerous to update all test
data to reflect this field, which would now be nil everywhere.
@atombender
Copy link
Contributor Author

Oh, and this fixes #54.

@atombender
Copy link
Contributor Author

@davecgh Thoughts? I was thinking of changing the nil functionality to disable all zero values, which will cover more use cases.

@kmudrick kmudrick mentioned this pull request Apr 6, 2018
@kmudrick
Copy link

kmudrick commented Apr 6, 2018

I really like the DisableNilValues option

@georgelesica-wf
Copy link

@davecgh do you think we could move this along? It would help us resolve stretchr/testify#288. Or, if it's not going to happen, we can just close that or maybe try to fix it on our end. Either way works for me.

@maxatome
Copy link

For the DisableNilValues I think #77 is better suited as it covers all zero cases.

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.

4 participants