Better docs and associated SUCCESS/FAILURE for process::ExitCode #48618
Conversation
rust-highfive
added
the
S-waiting-on-review
src/libstd/process.rs
Outdated
| /// the [`SUCCESS`] and [`FAILURE`] associated items. | ||
| /// | ||
| /// [`SUCCESS`]: #constant.SUCCESS | ||
| /// [`FAILURE`]: #constant.FAILURE |
There was a problem hiding this comment.
[01:29:50] std/process/struct.ExitCode.html:68: broken link fragment `#constant.SUCCESS` pointing to `std/process/struct.ExitCode.html`
[01:29:50] std/process/struct.ExitCode.html:68: broken link fragment `#constant.FAILURE` pointing to `std/process/struct.ExitCode.html`
[01:29:56] thread 'main' panicked at 'found some broken links', tools/linkchecker/main.rs:49:9
src/libstd/process.rs
Outdated
| #[cfg(target_arch = "wasm32")] | ||
| mod rawexit { | ||
| pub const SUCCESS: i32 = 0; | ||
| pub const FAILURE: i32 = 1; |
There was a problem hiding this comment.
do we still need this?
There was a problem hiding this comment.
Er I'm not sure I added this... This actually should be rejected by the tidy lint as well...
In any case seems fine to have if it's needed, we can clean it up later
There was a problem hiding this comment.
This private mod is originally from https://github.com/rust-lang/rust/pull/46479/files#diff-7dbed9a9f4aeab33f13e792758ac1ed5R13
Did you mean not have the wasm check? Or not have the mod? I can do either or both...
There was a problem hiding this comment.
Er sorry what I mean is that we have lint which forbids #[cfg] in most of the standard library in an attempt to centralize all platform-specific code to src/libstd/sys rather than having it sprinkled throughout the standard library itself.
It's ok to not fix this up here, but if you're feeling enterprising this would just involve moving the modules to src/libstd/sys and using constants from there rather than this location.
There was a problem hiding this comment.
I think the main question I had was "does wasm offer the standard constants" -- note though that this code is pre-existing, though it's been moved around, so the lint must not work that well...
| /// | ||
| /// **Warning**: While various forms of this were discussed in [RFC #1937], | ||
| /// it was ultimately cut from that RFC, and thus this type is more subject | ||
| /// to change even than the usual unstable item churn. |
There was a problem hiding this comment.
as an aside, I just ran into a place where I really wanted this today. I was already calling exit(1) -- so it seems silly not to provide a way to "return" that...
|
@scottmcm do you want to make the changes @alexcrichton suggested? |
Now begins the saga of fixing compilation errors on other platforms...
scottmcm
force-pushed
the
elaborate-exitcode
branch
from
61cc4f0
to
74c5c6e
Compare
scottmcm
changed the title
scottmcm
force-pushed
the
elaborate-exitcode
branch
from
8610f07
to
74c5c6e
Compare
scottmcm
changed the title
|
Ok, hopefully this is good now. Made a new tracking issue for the type, #48711 Travis job to see if I typo'd anything in wasm: https://travis-ci.org/rust-lang/rust/builds/348817793 |
|
I can't really comment on the |
|
@bors: r+ |
|
📌 Commit 74c5c6e has been approved by |
bors
added
S-waiting-on-bors
…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
Follow-up to #48497 (comment), since that PR was the minimal thing to unblock #48453 (comment).
r? @nikomatsakis