-
Notifications
You must be signed in to change notification settings - Fork 651
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
Rework build-setup | Add single-node CI #1282
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice.
968ae89
to
a37a935
Compare
97e8605
to
5996ec6
Compare
@jerryz123 Re-review request! |
scripts/build-setup.sh
Outdated
* ) | ||
error "invalid option $1" | ||
usage 1 ;; | ||
esac | ||
shift | ||
done | ||
|
||
if [ "$SKIP_CONDA" = false ]; then | ||
# check if the arg is found in the SKIP_LIST | ||
do_skip() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a bit confusing, since if do_skip
evaluates to true, the block is run.
Perhaps rename do_skip
to run_step
, where run_step
returns true if the ARG is not in the SKIP_LIST
.
scripts/build-setup.sh
Outdated
popd | ||
fi | ||
|
||
if do_skip "6"; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Add comment before each block specifying what the block does
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some changes to make the script easier to read.
Remove conda from build-toolchains-extra.sh
Updates the
build-setup.sh
script to initialize as much as possible (FireSim, FireMarshal, tags, etc). However, there is a new opt-out mechanism for power-users to disable different parts of the setup (this is not tested extensively). Additionally, this PR adds a new CI workflow where a persistent CI clone is set up and regressed on (in the future this will be used to regress on the tutorial setup).Related PRs / Issues:
Type of change:
Impact:
Contributor Checklist:
main
as the base branch?changelog:<topic>
label?changelog:
label?.conda-lock.yml
file if you updated the conda requirements file?Please Backport
?