refactor: invert conditionals for less nesting in includeScriptOutput
#12146
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- I didn't get a chance to review e5d8c53 before it was merged in, but it added more nesting when there was already some unnecessary nesting - invert the `boundaryID` conditional from there for less nesting - and invert the `boundaryNode` conditional as well for less nesting - less nesting = less code complexity - **NOTE**: there is a small semantic difference here from the previous code from 1f6b19f - when there is an error retrieving `boundaryNode`, the error will now be returned - previously it was not returned, but the purpose of the PR was to expose these errors as they are indicative of an underlying logic bug - so I think this is still within the intent of the PR, but tagging the author just in case Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
agilgur5
commented
Nov 4, 2023
| boundaryNode, err := woc.wf.Status.Nodes.Get(boundaryID) | ||
| if err != nil { | ||
| woc.log.Errorf("was unable to obtain node for %s", boundaryID) | ||
| return false, err |
There was a problem hiding this comment.
this would've returned false, nil before
juliev0
approved these changes
Dec 6, 2023
tczhao
pushed a commit
to tczhao/argo
that referenced
this pull request
Oct 2, 2024
…t` (argoproj#12146) Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
Modifications
invert the
boundaryIDconditional from there for less nestingand invert the
boundaryNodeconditional as well for less nestingNOTE: there is a small semantic difference here from the previous code from fix: add guard against NodeStatus. Fixes #11102 #11451
boundaryNode, the error will now be returnedVerification