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.

Sign up for GitHub

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

Jump to bottom

add comment to lto.rs #11701

Merged
merged 1 commit into from
Feb 11, 2023
Merged

add comment to lto.rs #11701

merged 1 commit into from
Feb 11, 2023

Conversation

Ethan-000
Copy link
Contributor

No description provided.

@rustbot
Copy link
Collaborator

rustbot commented Feb 11, 2023

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @epage (or someone else) soon.

Please see the contribution instructions for more information.

@rustbot rustbot added A-lto Area: link-time optimization S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 11, 2023
weihanglo
weihanglo reviewed Feb 11, 2023
View reviewed changes
src/cargo/core/compiler/lto.rs Outdated
@@ -14,6 +15,8 @@ use std::collections::hash_map::{Entry, HashMap};
/// `OnlyBitcode` | `-C linker-plugin-lto` | | ✓
/// `ObjectAndBitcode` | | ✓ | ✓
/// `OnlyObject` | `-C embed-bitcode=no` | ✓ |
///
/// See <https://doc.rust-lang.org/nightly/cargo/reference/profiles.html#lto>
Copy link
Member

Choose a reason for hiding this comment

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

Thanks. However, this doesn't look good in rustdoc.

image

To maintain readability in both rustdoc and raw content, instead of adding "LTO: Link Time Optimizations", I suggest simply give a link on LTO text, such as

- /// LTO: Link Time Optimizations
- /// Possible ways to run rustc and request various parts of LTO.
+ /// Possible ways to run rustc and request various parts of [LTO].
  /// ...
  /// ...
- /// See <https://doc.rust-lang.org/nightly/cargo/reference/profiles.html#lto>
+ /// [LTO]: https://doc.rust-lang.org/nightly/cargo/reference/profiles.html#lto

Copy link
Contributor Author

Choose a reason for hiding this comment

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

makes sense

Copy link
Contributor Author

@Ethan-000 Ethan-000 Feb 11, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

how to see how it looks like on rustdoc tho. is there a command to do this 🧐

Copy link
Member

Choose a reason for hiding this comment

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

cargo doc --document-private-items --no-deps --open and search lto.

BTW, could you help squash this pull request to one commit to keep the commit history clean?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure. but i don't know how to do it sorry. it seems i did something wrong 😭

Copy link
Member

Choose a reason for hiding this comment

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

You can reset to the latest commit on master branch and add your patch, as this patch is quite small. For example, git reset --hard origin/master

git reset tutorial

Copy link
Contributor Author

@Ethan-000 Ethan-000 Feb 11, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

this should be ok now. thanks for the help 🙏

@Ethan-000 Ethan-000 force-pushed the add_lto_comment branch 2 times, most recently from 98f4765 to c059820 Compare February 11, 2023 13:40
@rustbot rustbot added A-cli Area: Command-line interface, option parsing, etc. A-cli-help Area: built-in command-line help A-documenting-cargo-itself Area: Cargo's documentation labels Feb 11, 2023
@weihanglo weihanglo removed A-documenting-cargo-itself Area: Cargo's documentation A-cli Area: Command-line interface, option parsing, etc. A-cli-help Area: built-in command-line help labels Feb 11, 2023
weihanglo
weihanglo approved these changes Feb 11, 2023
View reviewed changes
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.

Thanks!

@weihanglo
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Feb 11, 2023

📌 Commit aefde60 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 Feb 11, 2023
@bors
Copy link
Contributor

bors commented Feb 11, 2023

⌛ Testing commit aefde60 with merge 7113896...

@bors
Copy link
Contributor

bors commented Feb 11, 2023

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

@bors bors merged commit 7113896 into rust-lang:master Feb 11, 2023
@Ethan-000 Ethan-000 deleted the add_lto_comment branch February 11, 2023 15:16
@weihanglo weihanglo mentioned this pull request Feb 13, 2023
Merged
weihanglo added a commit to weihanglo/rust that referenced this pull request Feb 13, 2023
@weihanglo
Update cargo
4c4b4c0 
10 commits in 82c3bb79e3a19a5164e33819ef81bfc2c984bc56..39c13e67a5962466cc7253d41bc1099bbcb224c3
2023-02-04 22:52:16 +0000 to 2023-02-12 02:01:08 +0000

- chore: Update to toml v0.6, toml_edit v0.18 (rust-lang/cargo#11618)
- doc: more doc comments and intra-doc links (rust-lang/cargo#11703)
- Deny warnings in CI, not locally (rust-lang/cargo#11699)
- add comment to lto.rs (rust-lang/cargo#11701)
- Re-export cargo_new::NewProjectKind as public (rust-lang/cargo#11700)
- Add '-C' flag for changing current dir before build (rust-lang/cargo#10952)
- `-Zrustdoc-scrape-example` must fail with bad build script (rust-lang/cargo#11694)
- Update CHANGELOG for 1.68 backports (rust-lang/cargo#11690)
- Update 1password to the version 2 CLI (rust-lang/cargo#11692)
- chore: autolabel more for `A-*` (rust-lang/cargo#11679)
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 14, 2023
@bors
Auto merge of rust-lang#107993 - weihanglo:update-cargo, r=weihanglo
c3c6d73 
Update cargo

10 commits in 82c3bb79e3a19a5164e33819ef81bfc2c984bc56..39c13e67a5962466cc7253d41bc1099bbcb224c3 2023-02-04 22:52:16 +0000 to 2023-02-12 02:01:08 +0000

- chore: Update to toml v0.6, toml_edit v0.18 (rust-lang/cargo#11618)
- doc: more doc comments and intra-doc links (rust-lang/cargo#11703)
- Deny warnings in CI, not locally (rust-lang/cargo#11699)
- add comment to lto.rs (rust-lang/cargo#11701)
- Re-export cargo_new::NewProjectKind as public (rust-lang/cargo#11700)
- Add '-C' flag for changing current dir before build (rust-lang/cargo#10952)
- `-Zrustdoc-scrape-example` must fail with bad build script (rust-lang/cargo#11694)
- Update CHANGELOG for 1.68 backports (rust-lang/cargo#11690)
- Update 1password to the version 2 CLI (rust-lang/cargo#11692)
- chore: autolabel more for `A-*` (rust-lang/cargo#11679)

r? `@ghost`
@ehuss ehuss added this to the 1.69.0 milestone Mar 9, 2023
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this pull request Apr 20, 2024
@bors
Auto merge of #107993 - weihanglo:update-cargo, r=weihanglo
02e04ef 
Update cargo

10 commits in 82c3bb79e3a19a5164e33819ef81bfc2c984bc56..39c13e67a5962466cc7253d41bc1099bbcb224c3 2023-02-04 22:52:16 +0000 to 2023-02-12 02:01:08 +0000

- chore: Update to toml v0.6, toml_edit v0.18 (rust-lang/cargo#11618)
- doc: more doc comments and intra-doc links (rust-lang/cargo#11703)
- Deny warnings in CI, not locally (rust-lang/cargo#11699)
- add comment to lto.rs (rust-lang/cargo#11701)
- Re-export cargo_new::NewProjectKind as public (rust-lang/cargo#11700)
- Add '-C' flag for changing current dir before build (rust-lang/cargo#10952)
- `-Zrustdoc-scrape-example` must fail with bad build script (rust-lang/cargo#11694)
- Update CHANGELOG for 1.68 backports (rust-lang/cargo#11690)
- Update 1password to the version 2 CLI (rust-lang/cargo#11692)
- chore: autolabel more for `A-*` (rust-lang/cargo#11679)

r? `@ghost`
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this pull request Apr 27, 2024
@bors
Auto merge of #107993 - weihanglo:update-cargo, r=weihanglo
654ca6d 
Update cargo

10 commits in 82c3bb79e3a19a5164e33819ef81bfc2c984bc56..39c13e67a5962466cc7253d41bc1099bbcb224c3 2023-02-04 22:52:16 +0000 to 2023-02-12 02:01:08 +0000

- chore: Update to toml v0.6, toml_edit v0.18 (rust-lang/cargo#11618)
- doc: more doc comments and intra-doc links (rust-lang/cargo#11703)
- Deny warnings in CI, not locally (rust-lang/cargo#11699)
- add comment to lto.rs (rust-lang/cargo#11701)
- Re-export cargo_new::NewProjectKind as public (rust-lang/cargo#11700)
- Add '-C' flag for changing current dir before build (rust-lang/cargo#10952)
- `-Zrustdoc-scrape-example` must fail with bad build script (rust-lang/cargo#11694)
- Update CHANGELOG for 1.68 backports (rust-lang/cargo#11690)
- Update 1password to the version 2 CLI (rust-lang/cargo#11692)
- chore: autolabel more for `A-*` (rust-lang/cargo#11679)

r? `@ghost`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@weihanglo weihanglo weihanglo approved these changes

Assignees

@epage epage

Labels
A-lto Area: link-time optimization S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.69.0
Development

Successfully merging this pull request may close these issues.
6 participants
@Ethan-000 @rustbot @weihanglo @bors @ehuss @epage