Skip to content

Conversation

nyurik
Copy link
Contributor

@nyurik nyurik commented Oct 7, 2025

implement #805

@max-sixty
Copy link
Collaborator

I very much agree with the value of compact snapshots

but I'm a bit worried about macro proliferation; that we have a different macro for each combination of settings. I was already a bit hesitant about assert_compact_json_snapshot, but IIRC @mitsuhiko was keener.

alternatives would include:

  • start with a wrapper in the crate being tested that ran the compact json and then passed to assert_snapshot
  • adding code to insta so we can set settings for how to serialize

@mitsuhiko do you have thoughts?

@nyurik
Copy link
Contributor Author

nyurik commented Oct 7, 2025

can we simply re-target assert_compact_json_snapshot to use this new renderer?

P.S. the fewer settings we have in insta, the better - conventions over configuration :)

@max-sixty
Copy link
Collaborator

can we simply re-target assert_compact_json_snapshot to use this new renderer?

but I think it would be backward incompat because it would require enabling a feature? (and we would want a new feature as it would be adding a dependency...)

@nyurik
Copy link
Contributor Author

nyurik commented Oct 7, 2025

unless someone wants to re-implement it in-place... Than again, I have no idea why insta wants to maintain a JSON rendering code rather than just use serde_json? Doesn't seem like a high value thing, esp considering that serde_json is looked at by so many more people...

@nyurik
Copy link
Contributor Author

nyurik commented Oct 7, 2025

P.S. technically adding a small mandatory dependency is not a biggie of course

@max-sixty
Copy link
Collaborator

Than again, I have no idea why insta wants to maintain a JSON rendering code rather than just use serde_json?

yeah, there's some modest history there; discussions are easy to grab

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