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

Various rustfmt and commenting changes #49969

Merged
merged 3 commits into from
Apr 18, 2018
Merged

Various rustfmt and commenting changes #49969

merged 3 commits into from
Apr 18, 2018

Conversation

mark-i-m
Copy link
Member

These are factored out of #49320

There aren't actually any changes in functionality, just rustfmt and doccomments.

@rust-highfive
Copy link
Collaborator

r? @estebank

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 14, 2018
@mark-i-m mark-i-m mentioned this pull request Apr 14, 2018
Closed
@bors
Copy link
Contributor

bors commented Apr 16, 2018

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

estebank
estebank reviewed Apr 16, 2018
View reviewed changes
Copy link
Contributor

@estebank estebank left a comment

Choose a reason for hiding this comment

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

LGTM overall, fix the merge conflict and r=me. That being said, I'd lean towards really small rustfmt PRs to minimize the likelihood of causing merge conflicts for open PRs.

src/librustc_driver/driver.rs
base.push(::rustc_trans_utils::link::default_output_for_target(session));
base.push(::rustc_trans_utils::link::default_output_for_target(
session,
));
Copy link
Contributor

Choose a reason for hiding this comment

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

this is an odd change

Copy link
Member Author

Choose a reason for hiding this comment

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

I just ran rustfmt on the file. I'm guessing it is avoiding >=100 chars per line...

src/librustc_driver/driver.rs
session.warn(&format!(
"dropping unsupported crate type `{}` for target `{}`",
*crate_type, session.opts.target_triple
));
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see this change as an improvement :-/

Copy link
Member Author

Choose a reason for hiding this comment

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

Eh, I could accept either way...

@mark-i-m
Copy link
Member Author

@estebank Thanks :) Rebased.

@estebank
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Apr 16, 2018

📌 Commit d984e4a has been approved by estebank

@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 Apr 16, 2018
@bors
Copy link
Contributor

bors commented Apr 17, 2018

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

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 17, 2018
@mark-i-m
Copy link
Member Author

@estebank rebased.

@estebank
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Apr 17, 2018

📌 Commit 787b705 has been approved by estebank

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 17, 2018
@bors
Copy link
Contributor

bors commented Apr 18, 2018

⌛ Testing commit 787b705 with merge dcb44ca...

bors added a commit that referenced this pull request Apr 18, 2018
@bors
Auto merge of #49969 - mark-i-m:allocator_fmt, r=estebank
dcb44ca 
Various rustfmt and commenting changes

These are factored out of #49320

There aren't actually any changes in functionality, just rustfmt and doccomments.
@bors
Copy link
Contributor

bors commented Apr 18, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: estebank
Pushing dcb44ca to master...

@bors bors merged commit 787b705 into rust-lang:master Apr 18, 2018
@bors bors mentioned this pull request Apr 18, 2018
Closed
5 tasks
@mark-i-m mark-i-m deleted the allocator_fmt branch November 14, 2018 16:40
@pnkfelix pnkfelix mentioned this pull request Dec 3, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@estebank estebank estebank left review comments

Assignees

@estebank estebank

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mark-i-m @rust-highfive @bors @estebank