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

Fixup for #22510 #22982

Closed
wants to merge 1 commit into from
Closed

Fixup for #22510 #22982

wants to merge 1 commit into from

Conversation

tbu-
Copy link
Contributor

@tbu- tbu- commented Mar 2, 2015

@huonw
Copy link
Member

huonw commented Mar 2, 2015

Could you adjust the commit message so that it stands alone more (e.g. "Revert incorrect usize -> u32 replacements from #22510")?

@@ -283,7 +283,7 @@ impl Builder {
// address at which our stack started).
let main = move || {
let something_around_the_top_of_the_stack = 1;
let addr = &something_around_the_top_of_the_stack as *const isize;
let addr = &something_around_the_top_of_the_stack as *const i32;
Copy link
Member

Choose a reason for hiding this comment

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

Why change this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's irrelevant what something_around_the_top_of_the_stack is, so i32 is just okay.

@Manishearth
Copy link
Member

Just need a small explanation there to confirm; though I suspect I understand what's going on. Also, agree with huon, a standalone message would be nice. Thanks!

@tbu-
Copy link
Contributor Author

tbu- commented Mar 3, 2015

@Manishearth Reworded the commit message as suggested.

@Manishearth
Copy link
Member

@bors: rollup r+

@bors
Copy link
Contributor

bors commented Mar 3, 2015

@bors r=Manishearth 22c204a

Manishearth added a commit to Manishearth/rust that referenced this pull request Mar 3, 2015
@bors
Copy link
Contributor

bors commented Mar 3, 2015

⌛ Testing commit 22c204a with merge c330a54...

@bors
Copy link
Contributor

bors commented Mar 3, 2015

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

@Manishearth
Copy link
Member

This already merged

415c42e , d18897c

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