Skip to content

Rustfmt-ing librustc_driver. #29033

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
Closed
wants to merge 3 commits into from
Closed

Rustfmt-ing librustc_driver. #29033

wants to merge 3 commits into from

Conversation

goyox86
Copy link
Contributor

@goyox86 goyox86 commented Oct 13, 2015

Hi Rustaceans!

This is the result of running latest rustfmt on librustc_driver!

This is a longer one!

//cc @nrc

@rust-highfive
Copy link
Contributor

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @arielb1 (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. 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.

@nrc
Copy link
Member

nrc commented Oct 13, 2015

r? @nrc

@rust-highfive rust-highfive assigned nrc and unassigned arielb1 Oct 13, 2015
nrc
nrc reviewed Oct 13, 2015
View reviewed changes
src/librustc_driver/driver.rs
if log_enabled!(::log::INFO) {
println!("Post-trans");
tcx.print_debug_stats();
}
Copy link
Member

Choose a reason for hiding this comment

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

We really need to fix this for large blocks...

@nrc
Copy link
Member

nrc commented Oct 13, 2015

Thanks for the PR, there's a few things to fix up. I also wonder what to do about the huge whitespace we've created for the big closures. We could wait until we fix rustfmt, we could land as is, or we could manually fixup and revisit once rustfmt is improved. Do you have a preference? I think I'd prefer the second option.

@goyox86
Copy link
Contributor Author

goyox86 commented Oct 14, 2015

Hey @nrc Manual fixups made and rustfmt issues reported, we should be good now.

@nrc
Copy link
Member

nrc commented Oct 14, 2015

Thanks for the changes!

@bors: r+

(no rollup because it's big and there was a compile bug in an earlier commit)

@bors
Copy link
Collaborator

bors commented Oct 14, 2015

📌 Commit 59a3518 has been approved by nrc

@bors
Copy link
Collaborator

bors commented Oct 14, 2015

⌛ Testing commit 59a3518 with merge 9bc674b...

@bors
Copy link
Collaborator

bors commented Oct 14, 2015

💔 Test failed - auto-mac-64-nopt-t

@nrc
Copy link
Member

nrc commented Oct 15, 2015

You'll need to add #![feature(custom_attribute)] and #![allow(unused_attributes)] to the crate root to pass tests.

@goyox86
Copy link
Contributor Author

goyox86 commented Oct 15, 2015

@nrc Oh thanks added!

@nrc
Copy link
Member

nrc commented Oct 15, 2015

@bors: r+

@bors
Copy link
Collaborator

bors commented Oct 15, 2015

📌 Commit 605fef2 has been approved by nrc

@bors
Copy link
Collaborator

bors commented Oct 16, 2015

⌛ Testing commit 605fef2 with merge 6f18bc4...

@bors
Copy link
Collaborator

bors commented Oct 16, 2015

💔 Test failed - auto-mac-64-opt

@alexcrichton
Copy link
Member

@bors: retry

On Thu, Oct 15, 2015 at 5:48 PM, bors [email protected] wrote:

[image: 💔] Test failed - auto-mac-64-opt
http://buildbot.rust-lang.org/builders/auto-mac-64-opt/builds/6749

—
Reply to this email directly or view it on GitHub
#29033 (comment).

Manishearth added a commit to Manishearth/rust that referenced this pull request Oct 16, 2015
@Manishearth
Rollup merge of rust-lang#29033 - goyox86:goyox86/rustfmting-librustc…
bc7a22d 
…_driver, r=nrc

Hi Rustaceans!

This is the result of running latest rustfmt on librustc_driver!

This is a longer one!

//cc @nrc
@Manishearth Manishearth mentioned this pull request Oct 16, 2015
Closed
@bors
Copy link
Collaborator

bors commented Oct 16, 2015

⌛ Testing commit 605fef2 with merge c4fbea8...

@bors
Copy link
Collaborator

bors commented Oct 16, 2015

💔 Test failed - auto-mac-64-opt

@alexcrichton
Copy link
Member

Looks like the error may not be spurious? Perhaps something was accidentally changed here?

@nrc
Copy link
Member

nrc commented Oct 18, 2015

/build/src/librustc_driver/driver.rs:759: line longer than 100 chars

tidy error, looks like we broke a line in driver.rs

@bors
Copy link
Collaborator

bors commented Oct 21, 2015

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

@goyox86 goyox86 force-pushed the goyox86/rustfmting-librustc_driver branch from 605fef2 to 9393587 Compare October 21, 2015 16:53
@bors
Copy link
Collaborator

bors commented Oct 23, 2015

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

@goyox86 goyox86 mentioned this pull request Nov 5, 2015
Merged
@goyox86
Copy link
Contributor Author

goyox86 commented Nov 5, 2015

Closing this in favor of #29650

@goyox86 goyox86 closed this Nov 5, 2015
bors added a commit that referenced this pull request Nov 11, 2015
@bors
Auto merge of #29650 - goyox86:goyox86/rustfmting-librustc_driverII, …
8c743a9 
…r=nikomatsakis

Hi Rustaceans!

This is the secong take on running latest rustfmt on librustc_driver!

All fixups made in #29033 were done (also rustfmt got better).

//cc @nrc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@nrc nrc

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@goyox86 @rust-highfive @nrc @bors @alexcrichton @arielb1