pipeline: execute them with set -eo pipefail#2423
pipeline: execute them with set -eo pipefail#2423aborrero merged 1 commit intochainguard-dev:mainfrom
Conversation
aborrero
force-pushed
the
arturo-24694-pipeline-execute
branch
from
92e0cf0 to
7895b36
Compare
|
Enabling the |
If a pipeline is 'cmd1 | cmd2' we want to fail the execution if cmd1 failed. Previous to this patch, we had a raising number of pipelines that are manually encoding 'set -eo pipefail' on each instance. Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero@chainguard.dev>
aborrero
force-pushed
the
arturo-24694-pipeline-execute
branch
from
7895b36 to
825e17f
Compare
|
I think the most invasive change is |
aborrero
changed the title
| xFlag := "" | ||
| if debugOption == 'x' { | ||
| xFlag = "x" | ||
| } |
There was a problem hiding this comment.
Nit: I think a better change would be to remove debugOption rune from the function signature and replace it with something like enableTracing bool. Call sites don't have to know about x/-xtrace.
| script := fmt.Sprintf(`set -e%c | ||
| xFlag := "" | ||
| if debugOption == 'x' { | ||
| xFlag = "x" |
There was a problem hiding this comment.
This is going to cause a lot of spam we don't need an debugs run with -x? Why would we enabled it for all pipelines? @smoser didn't we disable this behavior in the past?
There was a problem hiding this comment.
Previous to this patch we were already enabling -x for pipelines in debug mode no? the behavior should be unchanged in this patch.
There was a problem hiding this comment.
just seeing this now. I do find set -x an anti-pattern. it makes the output of better tests, that provide reasonable output all of a sudden unreadable. but, as aborrero pointed out, it already was.
4 participants
If a pipeline is 'cmd1 | cmd2' we want to fail the execution if cmd1 failed.
Previous to this patch, we had a raising number of pipelines that are manually encoding
'set -eo pipefail' on each instance.
Signed-off-by: Arturo Borrero Gonzalez arturo.borrero@chainguard.dev