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

Document that cargo test --release uses profile.bench #4650

Merged
merged 2 commits into from
Oct 26, 2017

Conversation

main--
Copy link
Contributor

@main-- main-- commented Oct 21, 2017

It certainly makes sense but it's still surprising behavior when the docs clearly state cargo bench = profile.bench, cargo test = profile.test. Had to dive into the code to figure this out.

@rust-highfive
Copy link

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

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@matklad
Copy link
Member

matklad commented Oct 22, 2017

Thanks!

@bors r+

@bors
Copy link
Contributor

bors commented Oct 22, 2017

📌 Commit 7dea9b2 has been approved by matklad

@bors
Copy link
Contributor

bors commented Oct 22, 2017

⌛ Testing commit 7dea9b2 with merge a786e037b0809b60e25643921ba1a9100a47e09b...

@bors
Copy link
Contributor

bors commented Oct 22, 2017

💔 Test failed - status-appveyor

@matklad
Copy link
Member

matklad commented Oct 22, 2017

@bors retry

@bors
Copy link
Contributor

bors commented Oct 22, 2017

⌛ Testing commit 7dea9b2 with merge 3e95c0f...

bors added a commit that referenced this pull request Oct 22, 2017
Document that cargo test --release uses profile.bench

It certainly makes sense but it's still surprising behavior when the docs clearly state `cargo bench` = `profile.bench`, `cargo test` = `profile.test`. Had to dive into the code to figure this out.
@bors
Copy link
Contributor

bors commented Oct 22, 2017

💔 Test failed - status-appveyor

@bors
Copy link
Contributor

bors commented Oct 23, 2017

⌛ Testing commit 7dea9b2 with merge 06f220c79972628eb050598840b41ae00469827d...

@bors
Copy link
Contributor

bors commented Oct 23, 2017

💔 Test failed - status-appveyor

@matklad
Copy link
Member

matklad commented Oct 26, 2017

Oh, could you also update https://github.com/rust-lang/cargo/blob/master/src/doc/book/src/reference/manifest.md?

Turns out, we have two copies of docs in the repo at the moment, and the must be kept in sync 🤷‍♂️ . See readme in docs folder for an explanation :)

@main--
Copy link
Contributor Author

main-- commented Oct 26, 2017

Done.

@matklad
Copy link
Member

matklad commented Oct 26, 2017

@bors r+

Thanks!

@bors
Copy link
Contributor

bors commented Oct 26, 2017

📌 Commit 4b8249f has been approved by matklad

@bors
Copy link
Contributor

bors commented Oct 26, 2017

⌛ Testing commit 4b8249f with merge e5562dd...

bors added a commit that referenced this pull request Oct 26, 2017
Document that cargo test --release uses profile.bench

It certainly makes sense but it's still surprising behavior when the docs clearly state `cargo bench` = `profile.bench`, `cargo test` = `profile.test`. Had to dive into the code to figure this out.
@bors
Copy link
Contributor

bors commented Oct 26, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: matklad
Pushing e5562dd to master...

@bors bors merged commit 4b8249f into rust-lang:master Oct 26, 2017
@ehuss ehuss added this to the 1.23.0 milestone Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants