search

mitsuhiko / insta

A snapshot testing library for rust
https://insta.rs
Apache License 2.0
2.15k stars 98 forks source link

Deprecate `--no-force-pass` #513

Closed max-sixty closed 1 month ago

max-sixty commented 3 months ago

This seems like a confusing name, and is dominated by the much clearer --check

max-sixty commented 3 months ago

Clears up any confusion from #512

mitsuhiko commented 1 month ago

I'm okay with that, but it's confusing that the underlying envvar that drives the feature is called INSTA_FORCE_PASS. Will this not cause further confusion?

max-sixty commented 1 month ago

I'm okay with that, but it's confusing that the underlying envvar that drives the feature is called INSTA_FORCE_PASS. Will this not cause further confusion?

Because the option in cargo insta is --check but the env var in insta is INSTA_FORCE_PASS?

I don't think they necessarily have to correspond 1:1 — cargo insta is a higher level UX, and --check does a couple of other things. So I see INSTA_FORCE_PASS as mostly an internal way for cargo-insta to say "we're handling failures at the end; keep going insta".

If you feel strongly then we could hide that option while still allowing it. Or some other alternative? But as #512 demonstrates I don't think the existing state is a nice clear interface...

mitsuhiko commented 1 month ago

I think it might be fine to just say --check is the way to go.

max-sixty commented 1 month ago

I think it might be fine to just say --check is the way to go.

OK, interpreting this to mean you're good with hiding --no-force-pass. Let me know (+ apologies for misunderstanding) if that's not correct!