getsentry / json-schema-diff

Diff changes between JSON schemas
https://docs.rs/json-schema-diff/
15 stars 2 forks source link

include input values in snapshot for a smooth review #28

Closed 6293 closed 1 year ago

untitaker commented 1 year ago

I would use this instead: https://insta.rs/docs/advanced/#custom-descriptions-and-infos

6293 commented 1 year ago

ty

untitaker commented 1 year ago

I believe that if you use description instead of info, the input data appears in cargo insta review but does not require you to have that data in the snapshot. That's what I was trying to achieve. But I also can see the value in having input/output appear next to each other in PR diffs. So I'm going to merge this anyway

6293 commented 1 year ago

info also appears in the review session:

Reviewing [1/3] json-schema-diff@0.1.5:
Snapshot file: tests/snapshots/test__from_fixtures@additional_properties__restrict.json.snap
Snapshot: from_fixtures@additional_properties__restrict
Source: tests/test.rs:12
Input file: tests/fixtures/additional_properties/restrict.json
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
Expression: diff
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
lhs:
  additionalProperties: true
rhs:
  additionalProperties: false
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
-old snapshot
+new results
────────────┬───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
    0     0 │ [
    1     1 │     Change {
    2       │-        path: ".<additionalProperties>",;
          2 │+        path: ".<additionalProperties>",
    3     3 │         change: TypeRemove {
    4     4 │             removed: String,
    5     5 │         },
    6     6 │     },
────────────┴───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────

  a accept     keep the new snapshot
  r reject     keep the old snapshot
  s skip       keep both for now
  i hide info  toggles extended snapshot info
  d hide diff  toggle snapshot diff
untitaker commented 1 year ago

yup that's fine, i was mostly focused on how to get it out of the snapshot files, but there is value in having the input data there

6293 commented 1 year ago

ah I didn't read the whole sentences well! Thank you for the clarification