You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
@scottmcm: I think it should at least say somewhere what that default is. Having it come from a derive(Default) on a c_int feels indirect to me as well, so perhaps it would make sense to define it manually in terms of ExitCode::SUCCESS instead, say -- that would also allow putting a doc-comment on the impl.
We should document that Default for ExitStatus is "process reported successful termination", ie the result of ExitCode::success().into(), and, implement it that way.
You'd have to remove the derive and use manual impl Default blocks.
That ends up with the docs for what Default means lower in the rustdoc
page, next to the place where the impl and its methods appear. It
might be better to just document it in the doc comment for the struct.
Also just to check, It's ExitCode::SUCCESS.into() right? success() doesn't seem to be a method and doesn't seem to be being added by that issue (or should I add that method?)
Also, should I open a PR (that is on top of the existing PR to actually add the default), or wait until the other is merged? Done the changes locally already
Also just to check, It's ExitCode::SUCCESS.into() right? success()
doesn't seem to be a method and doesn't seem to be being added by
that issue (or should I add that method?)
Sorry, I think that was me writing loosely. Yes, I mean te constant
SUCCESS I thijnk. You don't need to add anything.
Ian.
Note #114593, which is a bit of a swamp. I think changing the docs can be done (at the cost of adding to the set of things which the docs claim to be true but which are false on those broken platforms) but changing the implementation ("and implement it that way") is blocked.
I’ve already got a branch with this issue fixed on it - was waiting for the previous PR to merge and it now has. For some reason I forget to claim it though
It appears this has already been done - i mentioned a few comments back but I've already completed this - i was going to claim it but im awaiting an update on whether its already been finished
Activity
john-h-k commentedon Apr 27, 2023
Is this possible to currently do? Having checked out the linked issue it looks like it depends on
impl Default for ExitStatusexistingjyn514 commentedon Apr 27, 2023
Right, it needs #106425 to merge first.
john-h-k commentedon Apr 27, 2023
Alright 👍 . How does documenting work for a derived trait?
jyn514 commentedon Apr 27, 2023
You'd have to remove the derive and use manual
impl Defaultblocks.ijackson commentedon Apr 27, 2023
john-h-k commentedon Apr 27, 2023
Also just to check, It's
ExitCode::SUCCESS.into()right?success()doesn't seem to be a method and doesn't seem to be being added by that issue (or should I add that method?)Also, should I open a PR (that is on top of the existing PR to actually add the default), or wait until the other is merged? Done the changes locally already
ijackson commentedon Apr 27, 2023
u-na-gi commentedon Aug 22, 2023
@rustbot claim
ijackson commentedon Aug 22, 2023
Note #114593, which is a bit of a swamp. I think changing the docs can be done (at the cost of adding to the set of things which the docs claim to be true but which are false on those broken platforms) but changing the implementation ("and implement it that way") is blocked.
john-h-k commentedon Aug 22, 2023
I’ve already got a branch with this issue fixed on it - was waiting for the previous PR to merge and it now has. For some reason I forget to claim it though
bkaestner commentedon Oct 11, 2023
Documented as of a741a5a within 1.73 via #114864 or am I mistaken?
HerrCai0907 commentedon Oct 20, 2023
@rustbot claim
john-h-k commentedon Oct 20, 2023
It appears this has already been done - i mentioned a few comments back but I've already completed this - i was going to claim it but im awaiting an update on whether its already been finished