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

[beta] Abort instead of UB if promotion fails #52624

Merged
merged 1 commit into from
Jul 24, 2018
Merged

[beta] Abort instead of UB if promotion fails #52624

merged 1 commit into from
Jul 24, 2018

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Jul 22, 2018 •
edited
Loading

original PR: #52571 (not beta approved yet)

r? @nikomatsakis

cc @RalfJung

@rust-highfive
Copy link
Collaborator

warning Warning warning

  • Pull requests are usually filed against the master branch for this repo, but this one is against beta. Please double check that you specified the right target!

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 22, 2018
@RalfJung
Copy link
Member

RalfJung commented Jul 22, 2018 •
edited
Loading

@oli-obk the "uncondensed" diff view in GitHub says that there is a submodule change included here. Is that a GitHub UI thing or real?

(Also wtf is that "condensed"/"uncondensed" diff stuff...)

@oli-obk oli-obk force-pushed the promotion_abort_backport branch from fcb62de to 9e621bb Compare July 22, 2018 20:09
@oli-obk
Copy link
Contributor Author

oli-obk commented Jul 22, 2018

Apparently I fail at rebasing... not sure what happened there, fixed now

@RalfJung
Copy link
Member

Now where do we have to lobby GitHub to revert that change to hide submodule changes in the diff? In which universe is that even a good idea?^^

(Sorry, I will stop being off-topic now.)

@RalfJung
Copy link
Member

The corresponding #52571 just landed in master.

@kennytm kennytm added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 23, 2018
@nikomatsakis
Copy link
Contributor

@bors r+

cc @rust-lang/compiler -- I'm approving this for beta backport. It's small and removes any chance of promotion introducing UB.

@bors
Copy link
Contributor

bors commented Jul 24, 2018

📌 Commit 9e621bb has been approved by nikomatsakis

@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-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). labels Jul 24, 2018
@bors
Copy link
Contributor

bors commented Jul 24, 2018

⌛ Testing commit 9e621bb with merge 0aaa819...

bors added a commit that referenced this pull request Jul 24, 2018
@bors
Auto merge of #52624 - oli-obk:promotion_abort_backport, r=nikomatsakis
0aaa819 
[beta] Abort instead of UB if promotion fails

original PR: #52571 (not beta approved yet)

r? @nikomatsakis

cc @RalfJung
@bors
Copy link
Contributor

bors commented Jul 24, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: nikomatsakis
Pushing 0aaa819 to beta...

@bors bors merged commit 9e621bb into rust-lang:beta Jul 24, 2018
@oli-obk oli-obk deleted the promotion_abort_backport branch July 26, 2018 14:50
@pnkfelix pnkfelix mentioned this pull request Jul 26, 2018
Merged
@RalfJung RalfJung mentioned this pull request Aug 12, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@nikomatsakis nikomatsakis

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@oli-obk @rust-highfive @RalfJung @nikomatsakis @bors @kennytm