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

Refactor core::ops, move RangeArgument to libcore::ops #42269

Closed
wants to merge 7 commits into from
Closed

Refactor core::ops, move RangeArgument to libcore::ops #42269

wants to merge 7 commits into from

Conversation

clarfonthey
Copy link
Contributor

@clarfonthey clarfonthey commented May 27, 2017 •
edited
Loading

This is a redone version of most of #41460 that moves RangeArgument to core::ops. This also refactors core::ops into submodules so that it's a bit easier to tell where things are.

This relies on #42268.

@rust-highfive
Copy link
Collaborator

r? @aturon

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

@clarfonthey clarfonthey mentioned this pull request May 27, 2017
Closed
@Mark-Simulacrum
Copy link
Member

Looks like travis failed:

[00:01:37] error: expected identifier, found keyword `fn`
[00:01:37]    --> /checkout/src/libcore/ops/mod.rs:151:5
[00:01:37]     |
[00:01:37] 151 | mod fn;
[00:01:37]     |     ^^
[00:01:37] 
[00:01:37] error: aborting due to previous error
[00:01:37] 
[00:01:37] error: Could not compile `core`.

@carols10cents carols10cents added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label May 29, 2017
@bors
Copy link
Contributor

bors commented Jun 1, 2017

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

@Mark-Simulacrum
Copy link
Member

@clarcharr Looks like this is still waiting on some fixes from you... I guess still waiting on #42268 though.

@aidanhs
Copy link
Member

aidanhs commented Jun 8, 2017

r? @alexcrichton
Since you've been actively involved with the history here (#41460, #42268) I figure you may want to take a look at this one.

@clarcharr is it possible/does it make sense for you to separate out the two PRs? I realise it's probably a bit of a pain to unpick, but it looks like the other one may be stalled for now?

@aidanhs aidanhs assigned alexcrichton and unassigned aturon Jun 8, 2017
@clarfonthey
Copy link
Contributor Author

@aidanhs I will do that! it makes sense either way

@clarfonthey
Copy link
Contributor Author

Also I have to redo this anyway because of the Carrier -> Try move.

@clarfonthey clarfonthey closed this Jun 8, 2017
@clarfonthey clarfonthey mentioned this pull request Jun 8, 2017
Merged
bors added a commit that referenced this pull request Jun 14, 2017
@bors
Auto merge of #42523 - clarcharr:refactor_ops, r=brson
554c685 
Refactor ops.rs

This refactors ops.rs into several different modules internally, as the file has gotten quite big. None of these modules are actually exported, but this should make maintaining it much easier. I've avoided the ambition of exporting the modules because they can more easily be rearranged after this commit goes through, even though it'd be cool to potentially export the modules in the future.

I've separated the creation of each file into a separate commit so that this is easier to read.

Redone version of #42269 with the movement of `RangeArgument` moved.
@clarfonthey clarfonthey deleted the rangearg_core branch May 5, 2018 04:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@alexcrichton alexcrichton

Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@clarfonthey @rust-highfive @Mark-Simulacrum @bors @aidanhs @alexcrichton @carols10cents @aturon