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

Use ternary expressions with wantarray #26

Closed
wants to merge 1 commit into from

Conversation

@happy-barney
Copy link

@happy-barney happy-barney commented Apr 22, 2018

so finalize code doesn't need to be duplicated.

(pull request challenge 2018)

so finalize code doesn't need to be duplicated.

(pull request challenge 2018)
# NB: We don't check the return status on close(), since
# on failure it sets $?, which we then inspect for more
# useful information.

This comment has been minimized.

@jkeenan

jkeenan Jan 25, 2020
Collaborator

I oppose the deletion of the author's inline comment. I am now co-maintainer of this distribution and I found this comment helpful.

Copy link
Collaborator

@jkeenan jkeenan left a comment

"If it ain't broke, don't fix it." Although the changes requested make the code more compact, it's not necessarily more readable and I doubt it improves functionality. Unless @pjf expresses a desire to apply this p.r. in the next 30 days, I will reject it.

@jkeenan jkeenan self-assigned this Jan 25, 2020
@happy-barney
Copy link
Author

@happy-barney happy-barney commented Jan 25, 2020

Primary motivation behind this PR is to get rid of duplication of _process_child_error calls in form:

if (condition) {
   ... do something
   _process_child_error();
   return ...
} else {
   ... do something else
   _process_child_error();
   return ...
}

Such construct is prone to introduce bugs when something is introduced only in one branch, so although it doesn't introduce new functionality, it just makes it little bit less likely to introduce a buggy feature.

@jkeenan
Copy link
Collaborator

@jkeenan jkeenan commented Mar 5, 2020

Primary motivation behind this PR is to get rid of duplication of _process_child_error calls in form:

if (condition) {
   ... do something
   _process_child_error();
   return ...
} else {
   ... do something else
   _process_child_error();
   return ...
}

Such construct is prone to introduce bugs when something is introduced only in one branch, so although it doesn't introduce new functionality, it just makes it little bit less likely to introduce a buggy feature.

Sorry, I'm not persuaded that the benefit outweighs the risk.

Thank you very much.
Jim Keenan

@jkeenan jkeenan closed this Mar 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.