Skip to content

Fix copy-bulk.sh and optimise dependencies#42

Merged
Silic0nS0ldier merged 3 commits intomainfrom
jordan-mele_optimise-copying-fix-bashism
Dec 6, 2023
Merged

Fix copy-bulk.sh and optimise dependencies#42
Silic0nS0ldier merged 3 commits intomainfrom
jordan-mele_optimise-copying-fix-bashism

Conversation

@Silic0nS0ldier
Copy link
Copy Markdown
Contributor

@Silic0nS0ldier Silic0nS0ldier commented Dec 5, 2023

Fixes shell usage and optimises dependency logic (gains will be more noticeable with bazelbuild/bazel#20434).

Copy link
Copy Markdown
Contributor

@christianscott christianscott left a comment

Choose a reason for hiding this comment

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

Why do we need to remove pipefail?

Comment thread internal/npm_install/bulk_copy.sh Outdated
@Silic0nS0ldier
Copy link
Copy Markdown
Contributor Author

Why do we need to remove pipefail?

Because I wrote the script to use sh (POSIX shell) which does not support that option.

The suggestion of outright using bash would make it safe though.

@Silic0nS0ldier Silic0nS0ldier changed the title Remove pipefail bashism from POSIX shell script Fix copy-bulk.sh and optimise dependencies Dec 6, 2023
Copy link
Copy Markdown
Contributor

@christianscott christianscott left a comment

Choose a reason for hiding this comment

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

LGTM

@Silic0nS0ldier Silic0nS0ldier merged commit 4af5b4b into main Dec 6, 2023
@Silic0nS0ldier Silic0nS0ldier deleted the jordan-mele_optimise-copying-fix-bashism branch December 6, 2023 23:35
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.

2 participants