Improve development experience

[tamird]

See individual commits.

[mitsuhiko]

I'm not entirely opposed to this change, but in the past I decided intentionally against it so I can end up with different .lock files for insta and cargo-insta due to different MSRV support.

[max-sixty]

end up with different .lock files for insta and cargo-insta due to different MSRV support.

FWIW for dependencies that are only part of cargo-insta, having the crates as a workspace won't affect MSRVs. We do this in PRQL, where the CLI can be on latest but the library follows debian.

(but maybe the dependencies are overlapping...)

I do think a workspace is a much nicer development experience.

[tamird]

@mitsuhiko I don't quite understand the concern. Wouldn't the .lock file only be published for cargo-insta, not for insta? That's my understanding of the usual convention.

[max-sixty]

@mitsuhiko I don't quite understand the concern. Wouldn't the .lock file only be published for cargo-insta, not for insta? That's my understanding of the usual convention.

There is only a single Cargo.lock file for a workspace. Unless I'm really mistaken — currently the cargo-insta/Cargo.lock file does nothing. And we should remove it.

[tamird]

That's right. I removed it. But my point was that the Cargo.lock should not be published with the insta library, only the cargo-insta CLI.

[tamird]

Ah, I see what you mean now. Indeed this change means that the versioning restrictions that apply to insta will also apply to cargo-insta.

[tamird]

Ugh, this changed the current directory when the integration tests run, and they are not resilient to this. I'll wait for #412 to land first.

[max-sixty]

Ah, I see what you mean now. Indeed this change means that the versioning restrictions that apply to insta will also apply to cargo-insta.

Yes cool. Still a big fan of the change!

Do you think we can still get 1.61 tests to pass?

[tamird]

Yeah, they were passing before I split out #412.

[mitsuhiko]

FWIW I would prefer a workspace as well. I think the way it could be accomplished might be with manually maintaining a secondary Cargo.lock file and move it over in the tests. I am doing something similar for a Cargo.lock.msrv file in some libraries. Eg.: https://github.com/mitsuhiko/similar

[max-sixty]

Re the MSRV — FWIW in PRQL we use cargo msrv verify in our CI, which verifies that that the package is valid with its specified MSRV (e.g. here).

It allows different MSRVs by crate.

Relative to maintaining another Cargo.lock.msrv, it requires another dev dependency, but it has worked quite well for us over the past year.

[tamird]

Alright I've abandoned the virtual manifest for now. Instead I added cargo-insta and its integration tests to the root manifest workspace.

[tamird]

Re the MSRV — FWIW in PRQL we use cargo msrv verify in our CI, which verifies that that the package is valid with its specified MSRV (e.g. here).

It allows different MSRVs by crate.

Relative to maintaining another Cargo.lock.msrv, it requires another dev dependency, but it has worked quite well for us over the past year.

That won't work here because MSRV is 1.51 but the tests also need to run against 1.61.

@mitsuhiko I did what you suggested with Cargo.lock.msrv.

[tamird]

At long last this is green.

[mitsuhiko]

I'm inclined to merge this (thus the approval). One thing I still need to figure out is if the cargo-insta installation with lockfile continues to work as intended. In particular I'm not sure if cargo publish is clever enough to publish a rewritten version of the workspace lock with the package.

[mitsuhiko]

Validated: the lock file is handled correctly by cargo package.

[max-sixty]

That won't work here because MSRV is 1.51 but the tests also need to run against 1.61.

FYI this isn't how cargo msrv works — check out the PRQL workflows if you're interested to learn more.

Thanks for this PR!