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

Restrict the Termination impls to simplify stabilization #48497

Merged
merged 1 commit into from
Feb 28, 2018

Conversation

scottmcm
Copy link
Member

Make a minimal commitment in preparation for stabilization. More impls, or broader ones, are likely in future, but are not necessary at this time and are more controversial.

cc #48453 (comment)
r? @nikomatsakis

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 24, 2018
/// This is ridiculously unstable, as it's a completely-punted-upon part
/// of the `?`-in-`main` RFC. It's here only to allow experimenting with
/// returning a code directly from main. It will definitely change
/// drastically before being stabilized, if it doesn't just get deleted.
Copy link
Member

Choose a reason for hiding this comment

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

Huh. Maybe we should #[doc(hidden)] it?

@bors
Copy link
Contributor

bors commented Feb 24, 2018

☔ The latest upstream changes (presumably #48510) 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-review Status: Awaiting review from the assignee but also interested parties. labels Feb 24, 2018
@scottmcm scottmcm force-pushed the more-restricted-termination branch from 48d957e to bc4d805 Compare February 25, 2018 07:37
@scottmcm
Copy link
Member Author

@nikomatsakis: Rebased atop #48143

Make a minimal commitment for stabilization.  More impls are likely in future, but are not necessary at this time.
@scottmcm scottmcm force-pushed the more-restricted-termination branch from bc4d805 to e20f7b2 Compare February 25, 2018 07:51
@@ -1080,6 +1080,15 @@ impl fmt::Display for ExitStatus {
}
}

/// This is ridiculously unstable, as it's a completely-punted-upon part
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 really care, but I feel like this is overstating the case. Specifying an exit code doesn't seem so "wild and crazy" to me.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's not what I meant, but I can see what you mean after rereading. I'll make a follow-up PR weakening the language here.

I definitely agree that fn main() -> ExitCode is valuable. I only intended the statement to be about the precise form of this type, since it's not B-RFC-Approved and thus even more subject to change than usual.

@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Feb 26, 2018

📌 Commit e20f7b2 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 26, 2018
kennytm added a commit to kennytm/rust that referenced this pull request Feb 27, 2018
…n, r=nikomatsakis

Restrict the Termination impls to simplify stabilization

Make a minimal commitment in preparation for stabilization.  More impls, or broader ones, are likely in future, but are not necessary at this time and are more controversial.

cc rust-lang#48453 (comment)
r? @nikomatsakis
kennytm added a commit to kennytm/rust that referenced this pull request Feb 28, 2018
…n, r=nikomatsakis

Restrict the Termination impls to simplify stabilization

Make a minimal commitment in preparation for stabilization.  More impls, or broader ones, are likely in future, but are not necessary at this time and are more controversial.

cc rust-lang#48453 (comment)
r? @nikomatsakis
bors added a commit that referenced this pull request Feb 28, 2018
Rollup of 15 pull requests

- Successful merges: #48266, #48321, #48365, #48381, #48450, #48473, #48479, #48484, #48488, #48497, #48541, #48548, #48558, #48560, #48565
- Failed merges:
@bors bors merged commit e20f7b2 into rust-lang:master Feb 28, 2018
@scottmcm scottmcm deleted the more-restricted-termination branch February 28, 2018 19:31
alexcrichton added a commit to alexcrichton/rust that referenced this pull request Mar 7, 2018
…crichton

Better docs and associated SUCCESS/FAILURE for process::ExitCode

Follow-up to rust-lang#48497 (comment), since that PR was the minimal thing to unblock rust-lang#48453 (comment).

r? @nikomatsakis
@marieell
Copy link

marieell commented Jul 4, 2021

I know this is pretty old but I'm surprised there is a hard-coded English word (»Error«) in the output implemented here. That's obviously pretty bad for every non-English use-case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants