Skip to content

Change set +x to set -x in CI wasm steps #2094

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

Merged
merged 1 commit into from
Aug 2, 2025

Conversation

EliahKagan
Copy link
Member

These steps benefit from showing the cargo command that was run before the resulting build output. Most run multiple cargo build commands, sometimes in a loop, such that the output can only be properly understood if the commands are shown. set +x is the default (in general, as well as in GitHub Actions unless the value of shell is modified with -x or -o xtrace). It looks like set -x was already intended here. Work in #2093 confirms a practical benefit of -x for understanding these logs.

set +x was part of the original code of these CI steps when they were introduced in 0d4b804 (#735). Its use was preserved and expanded in several changes. In 44ff412 (#1668), set +x was preserved and also positioned where it would make sense for set -x to be. Thus, it appears this started as a small typo and gradually expanded through misreadings, including my own. This fixes that.

(See EliahKagan#74 for verification that set +x had no effect.)


This is its own PR rather than being done as part of #2093, because this already confers a benefit, whereas I don't know if or when #2093 will be ready.

These steps benefit from showing the `cargo` command that was run
before the resulting build output. Most run multiple `cargo build`
commands, sometimes in a loop, such that the output can only be
properly understood if the commands are shown. `set +x` is the
default (in general, as well as in GitHub Actions unless the value
of `shell` is modified with `-x` or `-o xtrace`). It looks like
`set -x` was already intended here. Work in GitoxideLabs#2093 confirms a
practical benefit of `-x` for understanding these logs.

`set +x` was part of the original code of these CI steps when they
were introduced in 0d4b804 (GitoxideLabs#735). Its use was preserved and
expanded in several changes. In 44ff412 (GitoxideLabs#1668), `set +x` was
preserved and also positioned where it would make sense for `set
-x` to be. Thus, it appears this started as a small typo and
gradually expanded through misreadings, including my own. This
fixes that.

(See #74 for verification that `set +x` had no effect.)
EliahKagan

This comment was marked as resolved.

@EliahKagan EliahKagan marked this pull request as ready for review August 2, 2025 21:42
@EliahKagan EliahKagan enabled auto-merge August 2, 2025 21:42
@EliahKagan EliahKagan merged commit 8f51845 into GitoxideLabs:main Aug 2, 2025
23 checks passed
@EliahKagan EliahKagan deleted the wasm-ci-xtrace branch August 2, 2025 21:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant