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

Move libXtest into libX/tests #41037

Merged
merged 1 commit into from Apr 6, 2017
Merged

Move libXtest into libX/tests #41037

merged 1 commit into from Apr 6, 2017

Conversation

ghost
Copy link

@ghost ghost commented Apr 3, 2017

This change moves:

  1. libcoretest into libcore/tests
  2. libcollectionstest into libcollections/tests

This is a follow-up to #39561.

r? @alexcrichton

@ghost
Copy link
Author

ghost commented Apr 3, 2017

Tidy failed with:

/home/stjepan/work/rust/src/libcore/tests/num/flt2dec/strategy/dragon.rs:49: platform-specific cfg: cfg!(target_env = "msvc")
/home/stjepan/work/rust/src/libcore/tests/num/flt2dec/estimator.rs:14: platform-specific cfg: cfg(not(target_os = "emscripten"))
thread 'main' panicked at 'some tidy checks failed', src/tools/tidy/src/main.rs:63
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

I'm not sure what's up with that... Any ideas?

@petrochenkov
Copy link
Contributor

petrochenkov commented Apr 3, 2017

@stjepang
Tidy checks that crates do not contain platform specific cfgs unless whitelisted.
You should probably search for libcoretest in tidy whitelists and whitelist it under the new location.

@alexcrichton
Copy link
Member

@borsd: delegate=stjepang

r=me as soon as tidy is fixed

@bors
Copy link
Contributor

bors commented Apr 3, 2017

✌️ @stjepang can now approve this pull request

@malbarbo
Copy link
Contributor

malbarbo commented Apr 3, 2017

@stjepang What do you think about striping the lib prefix in the projects directory names? This looks like an ancient convention...

@ghost
Copy link
Author

ghost commented Apr 3, 2017

@malbarbo Sounds like a good idea to me. I'm all for removing vestigial conventions that don't make sense at this time anymore. However, I wonder if there are traps to watch out for when doing such a massive rename operation.

Here's a list of the 54 subdirectories that start with 'lib': list
Do we want to strip 'lib' from all of them? Should we leave some of those alone?

Another thing that is slightly odd is that directories like libcore and libcollections lump *.rs files together with Cargo.toml. Do we want to follow conventions established by Cargo and move all those *.rs files into src subdirectory?

Is there any other "housekeeping" work that needs to be done?

@alexcrichton What are your thoughts on this?

@alexcrichton
Copy link
Member

Let's not rename things just yet, that's a much broader discussion. Let's stick to this PR just moving the tests to the appropriate locations.

This change moves:

1. `libcoretest` into `libcore/tests`
2. `libcollectionstest` into `libcollections/tests`

This is a follow-up to #39561.
@malbarbo
Copy link
Contributor

malbarbo commented Apr 3, 2017

@stjepang I would like to see all vestigial conventions vanish. You raised some interesting questions, what about open an specific issue to address these questions?

@alexcrichton You are right, that's a broader discussion. I think @stjepang (like me) was just asking an opinion, the intent was not to change this PR, sorry for being unclear.

@ghost
Copy link
Author

ghost commented Apr 3, 2017

@bors r=alexcrichton

@bors
Copy link
Contributor

bors commented Apr 3, 2017

📌 Commit 13c744f has been approved by stjepang

@bors
Copy link
Contributor

bors commented Apr 3, 2017

💡 This pull request was already approved, no need to approve it again.

@bors
Copy link
Contributor

bors commented Apr 3, 2017

📌 Commit 13c744f has been approved by stjepang

@ghost
Copy link
Author

ghost commented Apr 3, 2017

@bors r=alexcrichton

@bors
Copy link
Contributor

bors commented Apr 3, 2017

💡 This pull request was already approved, no need to approve it again.

@bors
Copy link
Contributor

bors commented Apr 3, 2017

📌 Commit 13c744f has been approved by alexcrichton

frewsxcv added a commit to frewsxcv/rust that referenced this pull request Apr 6, 2017
Move libXtest into libX/tests

This change moves:

1. `libcoretest` into `libcore/tests`
2. `libcollectionstest` into `libcollections/tests`

This is a follow-up to rust-lang#39561.

r? @alexcrichton
frewsxcv added a commit to frewsxcv/rust that referenced this pull request Apr 6, 2017
Move libXtest into libX/tests

This change moves:

1. `libcoretest` into `libcore/tests`
2. `libcollectionstest` into `libcollections/tests`

This is a follow-up to rust-lang#39561.

r? @alexcrichton
bors added a commit that referenced this pull request Apr 6, 2017
Rollup of 5 pull requests

- Successful merges: #40908, #41011, #41026, #41037, #41050
- Failed merges:
@bors bors merged commit 13c744f into rust-lang:master Apr 6, 2017
@ghost ghost deleted the move-libxtest branch April 6, 2017 08:43
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.

4 participants