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

doc: intra-doc links and doc comments for build script #12133

Merged
merged 1 commit into from
May 13, 2023

Conversation

weihanglo
Copy link
Member

What does this PR try to resolve?

More docs and intra-doc links for src/cargo/core/compiler/custom_build.rs

How should we test and review this PR?

cargo doc --document-private-items --no-deps --open

Then proofread it!

@rustbot
Copy link
Collaborator

rustbot commented May 12, 2023

r? @epage

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added A-build-scripts Area: build.rs scripts S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 12, 2023
Copy link
Contributor

@epage epage left a comment

Choose a reason for hiding this comment

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

Feel free to merge on my behalf whether you end up fixing it or not

src/cargo/core/compiler/custom_build.rs Outdated Show resolved Hide resolved
@@ -1,3 +1,36 @@
//! A module about how to execute a custom build script and parse its output.
Copy link
Contributor

Choose a reason for hiding this comment

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

Side note: If custom_build.rs is really about build scripts, should we make that more obvious, whether thats renaming this to build_script.rs or build_rs.rs?

Copy link
Member Author

Choose a reason for hiding this comment

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

Had the same thought. I could see custom build/build script/build.rs co-exist everywhere. build_script.rs gets my vote and I can send a PR fixing all occurrences then.

@weihanglo
Copy link
Member Author

@bors r=epage

@bors
Copy link
Contributor

bors commented May 13, 2023

📌 Commit 227b2e5 has been approved by epage

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 May 13, 2023
@bors
Copy link
Contributor

bors commented May 13, 2023

⌛ Testing commit 227b2e5 with merge f7d0c8b...

@bors
Copy link
Contributor

bors commented May 13, 2023

☀️ Test successful - checks-actions
Approved by: epage
Pushing f7d0c8b to master...

@bors bors merged commit f7d0c8b into rust-lang:master May 13, 2023
@weihanglo weihanglo deleted the buildscript-doc branch May 13, 2023 20:32
bors added a commit to rust-lang-ci/rust that referenced this pull request May 17, 2023
Update cargo

8 commits in 13413c64ff88dd6c2824e9eb9374fc5f10895d28..09276c703a473ab33daaeb94917232e80eefd628
2023-05-10 13:46:18 +0000 to 2023-05-16 21:43:35 +0000
- docs: Clarify that crates.io doesn't link to docs.rs right away. (rust-lang/cargo#12146)
- docs(ref): Clarify MSRV is generally minor (rust-lang/cargo#12122)
- Fix `check_for_file_and_add`'s check for conflict file (rust-lang/cargo#12135)
- Fixes: Incorrect document link (rust-lang/cargo#12143)
- doc: intra-doc links and doc comments for build script (rust-lang/cargo#12133)
- Add Cargo team charter. (rust-lang/cargo#12010)
- Remove useless drop of copy type (rust-lang/cargo#12136)
- Fix dep/feat syntax with hidden implicit optional dependencies (rust-lang/cargo#12130)

r? ghost
@ehuss ehuss added this to the 1.71.0 milestone May 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-build-scripts Area: build.rs scripts 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.

5 participants