vendor: add summary output for crates vendored#16646
vendor: add summary output for crates vendored#16646raushan728 wants to merge 3 commits intorust-lang:masterfrom
Conversation
… summary output.
rustbot
added
Command-vendor
S-waiting-on-review
|
r? @weihanglo rustbot has assigned @weihanglo. Use Why was this reviewer chosen?The reviewer was selected based on:
|
There was a problem hiding this comment.
While this change might be simple and straightforwards, our contributing guideline encourages starting from an issue and discussion first before posting a pull request: https://doc.crates.io/contrib/process/working-on-cargo.html#before-hacking-on-cargo
There was a problem hiding this comment.
Sorry about that i am still learning the workflow here. I'll make sure to start with an issue next time
There was a problem hiding this comment.
Adds a final status message to 'cargo vendor' showing the count of crates vendored and the destination directory. This improves consistency with other commands like 'cargo install'
I personally don't immediately see the value of consistency here. Other maintainers may have different opinions though.
There was a problem hiding this comment.
For example, count of binaries installed via cargo install is essentially as we might not want to accidentally install extra bins. But do people really know about how many dependencies they vendor? What is the actual summary people want to know when vendoring. This might be worthy some level of discussions.
There was a problem hiding this comment.
I thought a final confirmation would be helpful for large workspaces, but I'm open to suggestions if you think the summary should be different or is unnecessary
tests/testsuite/vendor.rs
Outdated
| Package::new("bar", "0.1.0").publish(); | ||
|
|
||
| p.cargo("vendor --respect-source-config") | ||
| .with_stderr_contains("[..]Vendored 1 crates into [..]vendor") |
There was a problem hiding this comment.
We recommend snapshot testing with with_stderr_data method as the primary way to assert snapshot. See https://doc.crates.io/contrib/tests/writing.html
There was a problem hiding this comment.
Thanks for the tip I've updated the test to use with_stderr_data and snapshot matching
|
hi @weihanglo fixed the tests and logic. also, sorry for skipping the issue process—i am still learning the workflow. plz take a look |
3 participants
Adds a final status message to 'cargo vendor' showing the count of crates vendored and the destination directory. This improves consistency with other commands like 'cargo install'