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

Move #[cfg(test)] modules into separate files to save recompiling the std crate #75979

Merged
merged 2 commits into from
Aug 31, 2020
Merged

Move #[cfg(test)] modules into separate files to save recompiling the std crate #75979

merged 2 commits into from
Aug 31, 2020

Conversation

tesuji
Copy link
Contributor

@tesuji tesuji commented Aug 27, 2020 •
edited
Loading

Implements an accepted proposal: rust-lang/compiler-team#344

Some notes for reviewers:

  • mod tests nested in mod foo in mod bar, I move foo to a new file, tests is a new file in foo: For example library/std/src/sys/sgx/abi/tls.rs
  • mod test (not mod tests) also is moved.
  • mod benches are moved.
  • mod tests is placed before any use statements: The topic is discussed in https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/Code.20Style.20process
  • Some files in cloudabi was changed too. But I notice copyright banners in those files, should we ping cloudabi people?
  • I formatted files after moving tests around. I think that may make it easier to review :p .
  • Some files don't need ignore-tidy-filelength anymore.

@tesuji tesuji mentioned this pull request Aug 27, 2020
Closed
@tesuji
Copy link
Contributor Author

tesuji commented Aug 27, 2020

This is highly conflict to merge with other PRs.
Can we close the bors tree in the meantime ?
cc @Mark-Simulacrum

@tesuji tesuji force-pushed the seprate-tests branch 2 times, most recently from 5b2cbad to 2af6064 Compare August 27, 2020 13:49
@tesuji tesuji marked this pull request as ready for review August 27, 2020 15:16
@tesuji
Copy link
Contributor Author

tesuji commented Aug 27, 2020

Nice, successfully compiled on x86_64-linux-gnu.
@rustbot modify labels: +S-waiting-on-review
r? @Mark-Simulacrum

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 27, 2020
@jyn514 jyn514 added A-testsuite Area: The testsuite used to check the correctness of rustc T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Aug 28, 2020
@Kixiron Kixiron mentioned this pull request Aug 28, 2020
Open
@tesuji tesuji mentioned this pull request Aug 28, 2020
Open
@tesuji tesuji changed the title Moving #[cfg(test)] modules into a separate files to save recompiling the std crate Move #[cfg(test)] modules into separate files to save recompiling the std crate Aug 28, 2020
@bors
Copy link
Contributor

bors commented Aug 29, 2020

☔ The latest upstream changes (presumably #72808) made this pull request unmergeable. Please resolve the merge conflicts.

@tesuji
Copy link
Contributor Author

tesuji commented Aug 29, 2020

Rebased.

Mark-Simulacrum
Mark-Simulacrum reviewed Aug 29, 2020
View reviewed changes
Copy link
Member

@Mark-Simulacrum Mark-Simulacrum left a comment

Choose a reason for hiding this comment

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

I would prefer to omit the cloudabi changes from this PR.

Since git and other tooling presumably won't be able to follow this extraction anyway, could we squash the formatting changes in as well? That'll avoid extra noise in the commit log.

src/tools/tidy/src/unit_tests.rs Show resolved Hide resolved
library/std/src/memchr.rs Show resolved Hide resolved
@Mark-Simulacrum Mark-Simulacrum added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 29, 2020
@tesuji
Copy link
Contributor Author

tesuji commented Aug 30, 2020

I would prefer to omit the cloudabi changes from this PR.

That's what I thought. Done now.

Since git and other tooling presumably won't be able to follow this extraction anyway, could we squash the formatting changes in as well? That'll avoid extra noise in the commit log.

That's why I prefer an infra member or compiler member doing this change instead of me.
I thought letting fmt commit alone will make reviewing easier. Because you may know some tools to check the diff.
Looks like you and I don't have any clue about those tools. So I merged the fmt change as you requested.

@rustbot modify labels: -S-waiting-on-author +S-waiting-on-review

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 30, 2020
tesuji added 2 commits August 31, 2020 02:56
@tesuji
tidy: add new exceptions and remove std from skip list
db6cbfc 
Also doing fmt inplace as requested.
@tesuji
std: move "mod tests/benches" to separate files
a4e926d 
Also doing fmt inplace as requested.
@Mark-Simulacrum
Copy link
Member

@bors r+

Okay, seems good.

@bors
Copy link
Contributor

bors commented Aug 31, 2020

📌 Commit a4e926d has been approved by Mark-Simulacrum

@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 Aug 31, 2020
@bors
Copy link
Contributor

bors commented Aug 31, 2020

⌛ Testing commit a4e926d with merge 45f638b...

@bors
Copy link
Contributor

bors commented Aug 31, 2020

☀️ Test successful - checks-actions, checks-azure
Approved by: Mark-Simulacrum
Pushing 45f638b to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 31, 2020
@bors bors mentioned this pull request Aug 31, 2020
Merged
@bors bors merged commit 45f638b into rust-lang:master Aug 31, 2020
This was referenced Aug 31, 2020
Merged
Merged
Merged
Closed
Merged
Closed
@tesuji tesuji deleted the seprate-tests branch September 1, 2020 00:30
@jethrogb jethrogb mentioned this pull request May 7, 2021
Merged
jethrogb pushed a commit to jethrogb/rust that referenced this pull request May 7, 2021
Rearrange SGX split module files
bfa8484 
In rust-lang#75979 several inlined modules were split out into multiple files.
This PR keeps the multiple files but moves a few things around to
organize things in a coherent way.
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request May 7, 2021
@Dylan-DPC
Rollup merge of rust-lang#85030 - jethrogb:jb/sgx-rearrange-files, r=…
bd37dbe 
…nagisa

Rearrange SGX split module files

In rust-lang#75979 several inlined modules were split out into multiple files.
This PR keeps the multiple files but moves a few things around to
organize things in a coherent way.
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request May 7, 2021
@Dylan-DPC
Rollup merge of rust-lang#85030 - jethrogb:jb/sgx-rearrange-files, r=…
bfa80e5 
…nagisa

Rearrange SGX split module files

In rust-lang#75979 several inlined modules were split out into multiple files.
This PR keeps the multiple files but moves a few things around to
organize things in a coherent way.
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request May 7, 2021
@Dylan-DPC
Rollup merge of rust-lang#85030 - jethrogb:jb/sgx-rearrange-files, r=…
62b68f9 
…nagisa

Rearrange SGX split module files

In rust-lang#75979 several inlined modules were split out into multiple files.
This PR keeps the multiple files but moves a few things around to
organize things in a coherent way.
@cuviper cuviper added this to the 1.48.0 milestone Nov 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@petrochenkov petrochenkov petrochenkov left review comments

@Mark-Simulacrum Mark-Simulacrum Mark-Simulacrum left review comments

Assignees

@Mark-Simulacrum Mark-Simulacrum

Labels
A-testsuite Area: The testsuite used to check the correctness of rustc merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.48.0
Development

Successfully merging this pull request may close these issues.
7 participants
@tesuji @bors @Mark-Simulacrum @petrochenkov @cuviper @jyn514 @rustbot