Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add warning if potentially-scrapable examples are skipped due to dev-dependencies #11503

Merged
merged 2 commits into from
Dec 23, 2022

Conversation

willcrichton
Copy link
Contributor

What does this PR try to resolve?

Another point of feedback I've received on the scrape-examples feature is that the dev-dependency situation is quite confusing and subtle. To make users more aware of the issue, I added a warning where Cargo will alert users when examples are skipped due to a dev-dependency requirement, along with proposing a fix.

How should we test and review this PR?

The test docscrape::no_scrape_with_dev_deps has been updated to reflect this new warning.

r? @weihanglo

(PS thank you for the reviews Weihang. I know I'm doing lots of little patches right now to get this feature finalized. If you want to share the reviewing burden on scrape-examples with anyone else, let me know!)

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 20, 2022
@weihanglo
Copy link
Member

Sounds reasonable but not sure if it will become too verbose. Could we have a link to the original feedback as a reference?

@willcrichton
Copy link
Contributor Author

Sure, for instance here's a user whose examples weren't being scraped until they added this configuration: https://www.reddit.com/r/rust/comments/zosle6/feedback_requested_rustdocs_scraped_examples/j0qnmu5/

Right now users would only encounter this message if they enable scraping with -Zrustdoc-scrape-examples. Once this feature becomes more well-known and stabilized, I would be fine with changing to a terser message, perhaps with just a link to the Cargo book.

Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sense. Thank you for posting this!

Just some style suggestions. Feel free to let me know by giving a thumb down to this comment. I'll then merge it as-is 👻

src/cargo/ops/cargo_compile/unit_generator.rs Outdated Show resolved Hide resolved
src/cargo/ops/cargo_compile/unit_generator.rs Outdated Show resolved Hide resolved
src/cargo/ops/cargo_compile/unit_generator.rs Outdated Show resolved Hide resolved
@willcrichton
Copy link
Contributor Author

Just incorporated your suggestions @weihanglo.

@weihanglo
Copy link
Member

Thanks!

@bors r+

@bors
Copy link
Collaborator

bors commented Dec 22, 2022

📌 Commit 1c4065c has been approved by weihanglo

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 22, 2022
@bors
Copy link
Collaborator

bors commented Dec 22, 2022

⌛ Testing commit 1c4065c with merge 2a4a9b4...

@bors
Copy link
Collaborator

bors commented Dec 23, 2022

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing 2a4a9b4 to master...

@bors bors merged commit 2a4a9b4 into rust-lang:master Dec 23, 2022
bors pushed a commit to rust-lang-ci/rust that referenced this pull request Dec 23, 2022
7 commits in c994a4a638370bc7e0ffcbb0e2865afdfa7d4415..2381cbdb4e9b07090f552d34a44a529b6e620e44
2022-12-18 21:50:58 +0000 to 2022-12-23 12:19:27 +0000

- fix: deduplicate dependencies by artifact target (rust-lang/cargo#11478)
- Add warning if potentially-scrapable examples are skipped due to dev-dependencies (rust-lang/cargo#11503)
- Don't scrape examples from library targets by default (rust-lang/cargo#11499)
- Stabilize terminal-width (rust-lang/cargo#11494)
- Make sure that hash of `SourceId` is stable (rust-lang/cargo#11501)
- Use workspace lockfile when running `cargo package` and `cargo publish` (rust-lang/cargo#11477)
- Show `--help` if there is no man page for subcommand (rust-lang/cargo#11473)
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 23, 2022
Update cargo

7 commits in c994a4a638370bc7e0ffcbb0e2865afdfa7d4415..2381cbdb4e9b07090f552d34a44a529b6e620e44 2022-12-18 21:50:58 +0000 to 2022-12-23 12:19:27 +0000

- fix: deduplicate dependencies by artifact target (rust-lang/cargo#11478)
- Add warning if potentially-scrapable examples are skipped due to dev-dependencies (rust-lang/cargo#11503)
- Don't scrape examples from library targets by default (rust-lang/cargo#11499)
- Stabilize terminal-width (rust-lang/cargo#11494)
- Make sure that hash of `SourceId` is stable (rust-lang/cargo#11501)
- Use workspace lockfile when running `cargo package` and `cargo publish` (rust-lang/cargo#11477)
- Show `--help` if there is no man page for subcommand (rust-lang/cargo#11473)

r? `@ghost`
@ehuss ehuss added this to the 1.68.0 milestone Dec 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants