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

Remove do #11868

Merged
merged 11 commits into from
Jan 29, 2014
Merged

Remove do #11868

merged 11 commits into from
Jan 29, 2014

Conversation

bytbox
Copy link
Contributor

@bytbox bytbox commented Jan 28, 2014

Fixes #10815.

@bytbox
Copy link
Contributor Author

bytbox commented Jan 28, 2014

Wheee!

This needs rebasing of course, but with so many files affected, it'll need rebasing every 12 hours anyway, so I figured I'd go ahead and submit. Any major issues? (It does compile and stuff, so I'm mainly asking about the documentation, which I had to mangle a bit - see 03f9eb1)

@emberian
Copy link
Member

Awesome! Looks good to me, r+ with rebase

@bytbox
Copy link
Contributor Author

bytbox commented Jan 28, 2014

Rebased.

@bytbox
Copy link
Contributor Author

bytbox commented Jan 28, 2014

Hmmm... looks like it tried to pull and test an older version than I thought was in this pull request.

@alexcrichton
Copy link
Member

I think that a commit snuck in after this was rebased and before it was merged that introduced some do-notation.

@bytbox
Copy link
Contributor Author

bytbox commented Jan 28, 2014

Yeah, that's it: e901c4c. Rebasing and fixing.

@bytbox
Copy link
Contributor Author

bytbox commented Jan 28, 2014

Ok, it's probably good now - running tests on my box, but that's about 4 times as slow as bors, so if you want to optimistically tell bors to retry, it should work.

@brson
Copy link
Contributor

brson commented Jan 28, 2014

Extreme kudos! 🏄

@bytbox
Copy link
Contributor Author

bytbox commented Jan 29, 2014

@alexcrichton Bors seems to have missed your comment? (Or I just don't understand how it works, but the dashboard isn't updating for the PR.)

@huonw
Copy link
Member

huonw commented Jan 29, 2014

@bytbox looks like yet another instance of the confusion caused by github ordering commits chronologically, rather than topologically (that is, the HEAD commit wasn't listed last, and bors only looks at HEAD).

Silly GitHub! :(

@alexcrichton
Copy link
Member

Oops, sorry @bytbox! I should have been watching the queue and saw that this didn't go through...

@pnkfelix
Copy link
Member

Isn't this going to have a better chance of eventually landing if we do it piecemeal? I.e. instead of trying to atomically land the removal-of-usages and the removal-of-support-in-rustc in one PR, first get in as many removal-of-usages as possible in one PR and do the removal of support-in-rustc in a separate one?

Maybe people really are concerned about do's sneaking in during the lag time between separate PRs. Anyway, just a thought.

@huonw
Copy link
Member

huonw commented Jan 29, 2014

Can we just try to deal with that via review? Also the most recent build bootstrapped but failed on android for reasons that may be fixed by #11903.

(Also, I'd guess that people very rarely add dos now that it is specific to procs.)

@bytbox
Copy link
Contributor Author

bytbox commented Jan 29, 2014

Did another rebase/test cycle and confirmed that this is still safe to merge, so (assuming #11903 did in fact fix the android build issue) somebody should poke bors I guess. @alexcrichton

bors added a commit that referenced this pull request Jan 29, 2014
@bors bors closed this Jan 29, 2014
@bors bors merged commit a6867e2 into rust-lang:master Jan 29, 2014
bors added a commit that referenced this pull request Feb 19, 2014
The 'do' keyword was deprecated in 0.10 #11868 , and is keep as
reserved keyword  #12157 .

So the tutorial part about it doesn't make sense.
The spawning explanation was move into '15.2 Closure compatibility'.
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 1, 2023
Fix `box_default` behaviour with empty `vec![]` coming from macro arg

Fix rust-lang#11868

changelog: [`box_default`]: do not warn on `Box::new(vec![])` if the `vec![]` comes from a macro argument
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.

RFC: remove do
7 participants