feat(cli): Generate rustdoc automatically #80
Conversation
This deviates from the original plan to allow for running against a workspace. This mirrors cargo's git dependency logic. Fixes obi1kenobi#52
src/main.rs
Outdated
| let rustdoc = dump::RustDoc::new().deps(false).silence(false); | ||
|
|
||
| let rustdoc_paths = | ||
| if let Some(current_rustdoc_path) = args.current_rustdoc_path.as_deref() { | ||
| vec![( | ||
| loader.load_rustdoc("<unknown>")?, | ||
| loader.load_rustdoc(&rustdoc, "<unknown>")?, | ||
| current_rustdoc_path.to_owned(), | ||
| )] |
There was a problem hiding this comment.
If I'm reading this correctly, it seems like this is always generating new rustdoc JSON since new line 71 is always hit, even if there's a current_rustdoc_path set. That doesn't seem right to me?
There was a problem hiding this comment.
line 71 just creates the rustdoc wrapper. The active loader is what determines whether rustdoc is generated or not.
There was a problem hiding this comment.
Changing RustDoc to RustDocCommand helps a lot here, thanks! To simplify the thought process of the next person to come across this, consider leaving a comment next to let rustdoc mentioning that the command is prepared but not executed, and that the loader gets to choose whether to run it or not.
There was a problem hiding this comment.
I renamed the variable to clarify this is the command. This should clarify that it still needs a call done on it to actually execute.
|
All this looks good, my remaining comments are nits that I feel like are pretty straightforward and don't need an in-depth review. If you'd like to merge this, go for it 👍 If you'd prefer to keep polishing (docs, CLI help text, etc.) on this branch before merging, that's fine by me too. |
I lean towards smaller, independent iterations rather than upfront perfection. Feel free to fix or open issues in anything you see. Otherwise, I'll probably move on to git support and then registry support |
The original command-line will work as usual but now
--current, it will infer the packages like any other cargo command--baseline, you can specify--baseline-rootand it will search it for relevant manifests for what is in currentclap is enforcing the relationships to make this all work.
I've played with integration tests for this but we'll have to decide how well we want to handle that going forward. I was originally going to do integration tests against all of cargo-semverver's test cases but it was pretty slow :). I did hand test by having two clap directories set to different commits.
--baseline-gitwill build on this, we'll just clone first.--baseline-versionwill require a bit more work.This is best review per-commit to see how it all builds up
Fixes #52