Skip to content

Revert "Partial Fix Issue 16615 - convert os pid to std.process Pid"#5456

Merged
dlang-bot merged 1 commit intomasterfrom
revert-5086-process_enhancement
Jun 9, 2017
Merged

Revert "Partial Fix Issue 16615 - convert os pid to std.process Pid"#5456
dlang-bot merged 1 commit intomasterfrom
revert-5086-process_enhancement

Conversation

@CyberShadow
Copy link
Member

Reverts #5086

See #5086 (comment) for rationale.

@dlang-bot
Copy link
Contributor

Fix Bugzilla Description
16615 std.process is missing functionality for child processes

Copy link
Contributor

@wilzbach wilzbach left a comment

Choose a reason for hiding this comment

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

I am just following the discussion passively, but it's clear that that it's a very active open discusison, so reverting it & continuing the discussion sounds very reasonable.

@dlang-bot dlang-bot merged commit 9bfc821 into master Jun 9, 2017
@andralex
Copy link
Member

andralex commented Jun 9, 2017

I would appreciate if my decisions weren't undone without my approval. Thanks.

@wilzbach wilzbach deleted the revert-5086-process_enhancement branch June 9, 2017 17:05
@wilzbach
Copy link
Contributor

wilzbach commented Jun 9, 2017

I would appreciate if my decisions weren't undone without my approval. Thanks.

Sorry, I thought it's the general policy that when a controversial PR was merged without appropriate review and/or someone pointing out non-trivial issues that we revert the change until there's a consensus on the best course of action?

@CyberShadow
Copy link
Member Author

I would appreciate if my decisions weren't undone without my approval. Thanks.

Considering that reviewer time is scarce, I think it's understandable when occasionally something is merged with a superficial review by someone without experience in said area of the code base. However, the consequence of that is that we should be OK with reverting PRs as soon as it becomes obvious that there are more issues that need additional scrutiny post merge, which granted should've been raised during review in a perfect world. I think reverting early is a good idea, as it avoids the risk of an unfinished PR making it into the stable branch, in which case it takes more work of getting it out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants