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

rustbuild: use Display for exit status instead of Debug, see #74832 for justification #74841

Merged
merged 3 commits into from
Jul 28, 2020

Conversation

infinity0
Copy link
Contributor

No description provided.

Unverified

This user has not yet uploaded their public signing key.
…g#74832 for justification
@rust-highfive
Copy link
Collaborator

r? @Mark-Simulacrum

(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 Jul 27, 2020
@Mark-Simulacrum
Copy link
Member

r=me, but looks like ci is failing?

@infinity0
Copy link
Contributor Author

@Mark-Simulacrum it's because Display isn't implemented for Result (it's implemented for both ExitStatus and io::Error).

It would be easy to implement it and I could do it as part of the PR (it would have to come in two parts since it doesn't exist in stage0), however was there a reason it wasn't already implemented? Seems like such an obvious thing to have.

@infinity0
Copy link
Contributor Author

Display is not implemented for Option either so I figure this is some sort of libraries design decision, I'll update this PR to case-switch over the possibilities.

@Mark-Simulacrum
Copy link
Member

Yes, lack of Display impls for Result and Option is intentional - there's no universal user facing representation of either.

@bors r+

@bors
Copy link
Contributor

bors commented Jul 27, 2020

📌 Commit 3dcab29 has been approved by Mark-Simulacrum

@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 Jul 27, 2020
@infinity0
Copy link
Contributor Author

@Mark-Simulacrum I decided to just rewrite that section since it was repetitive and also had bugs (e.g. if you set both on-fail and RUSTC_PRINT_STEP_TIMINGS it wouldn't work correctly). It also allows us to get rid of the slightly messy status_code function.

@Mark-Simulacrum
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Jul 27, 2020

📌 Commit e7089a9 has been approved by Mark-Simulacrum

@bors
Copy link
Contributor

bors commented Jul 28, 2020

⌛ Testing commit e7089a9 with merge 1454bbd...

@bors
Copy link
Contributor

bors commented Jul 28, 2020

☀️ Test successful - checks-actions, checks-azure
Approved by: Mark-Simulacrum
Pushing 1454bbd to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 28, 2020
@bors bors merged commit 1454bbd into rust-lang:master Jul 28, 2020
@infinity0 infinity0 deleted the fix-exec branch July 28, 2020 20:12
@cuviper cuviper added this to the 1.47.0 milestone May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. 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.

5 participants