Skip to content

style nitpicks in rustc and libcollections #22539

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

Merged
merged 1 commit into from
Feb 25, 2015

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Feb 19, 2015

I tried to follow the style guide as much as possible. This is just from some random readings of the code, so no guarantees on completeness, even in the edited files.

@rust-highfive
Copy link
Contributor

r? @alexcrichton

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

new_pat
})
}
Some(const_expr) => const_expr_to_pat(self.tcx,
Copy link
Member

Choose a reason for hiding this comment

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

I think that this may wish to retain the outer block because it can be confusing if you expect the indented code below to be the body of the arm but it's in fact a closure.

@alexcrichton
Copy link
Member

A few small nits here and there, but overall looks great, thanks! (also needs a rebase)

@oli-obk oli-obk force-pushed the style_nitpicks branch 2 times, most recently from 3f66b8c to 5590326 Compare February 20, 2015 09:02

ty::BoundSized => { }
_ => Ok(If(types)),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this could allow for future code to be missed, should i create a manual fallback with all possible cases instead?

Copy link
Member

Choose a reason for hiding this comment

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

Yes keeping an explicit exhaustive match should be done wherever possible .

@alexcrichton
Copy link
Member

Sorry for being a little slow, but needs a rebase now.

@oli-obk oli-obk force-pushed the style_nitpicks branch 2 times, most recently from e4c269c to a343202 Compare February 23, 2015 08:41
@oli-obk
Copy link
Contributor Author

oli-obk commented Feb 23, 2015

nits addressed and rebased

@alexcrichton
Copy link
Member

@bors: r+ 1ae4bbffc668ba25d9e482d7b17a8378c4d71708 rollup

@Manishearth
Copy link
Member

(Needs rebase; removed from rollup)

@oli-obk
Copy link
Contributor Author

oli-obk commented Feb 24, 2015

rebased, compilation ok, checks running

@oli-obk
Copy link
Contributor Author

oli-obk commented Feb 24, 2015

checks are also ok

@Manishearth
Copy link
Member

Broke again, rollup just landed. So sorry!

Ping me when you get it rebased again.

@oli-obk
Copy link
Contributor Author

oli-obk commented Feb 24, 2015

np, trivial rebase, checks running

@@ -1692,7 +1666,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
self.tcx().sess.bug(
&format!(
"asked to assemble constituent types of unexpected type: {}",
t.repr(self.tcx()))[]);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

[] caused warnings

@oli-obk
Copy link
Contributor Author

oli-obk commented Feb 24, 2015

building found two warnings

@Manishearth
Copy link
Member

Okay, you can fix them and just quickly test if they don't fail. Or I can r+ this now, either works.

@oli-obk
Copy link
Contributor Author

oli-obk commented Feb 24, 2015

they are fixed

@oli-obk
Copy link
Contributor Author

oli-obk commented Feb 24, 2015

and make check ran through

@Manishearth
Copy link
Member

@bors: r+ 0bea550 rollup

Manishearth added a commit to Manishearth/rust that referenced this pull request Feb 24, 2015
 I tried to follow [the style guide][1] as much as possible. This is just from some random readings of the code, so no guarantees on completeness, even in the edited files.

[1]: http://aturon.github.io/style/README.html
@bors bors merged commit 0bea550 into rust-lang:master Feb 25, 2015
@oli-obk oli-obk deleted the style_nitpicks branch February 26, 2015 08:10
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.

5 participants