-
-
Notifications
You must be signed in to change notification settings - Fork 6
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
add CI to build, check fmt and use in example app #6
Conversation
ugh, forgot about my own todos, sorry |
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 looks great! I left a few suggestions but those can be done as follow-up PRs. Feel free to merge this PR after rebasing (if you wish to do that).
run: | | ||
sudo apt-get update && sudo apt-get install libssl-dev | ||
rustup target add ${{ env.CORE_TARGET }} | ||
cargo install cargo-generate |
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 can take a while to compile (and maybe annoying to compile on Windows). I think it may be cheaper to git clone
the app-template and then use sed
or similar to update placeholders like {crate_name}
.
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.
aaah good point. we could just do this with a second patch. Pr #2 in 3..2..1.. :)
.github/workflows/ci.yml
Outdated
#- nightly | ||
os: | ||
- ubuntu-latest | ||
# FIXME: Reenable macOS, Windows, and nightly once demt-test is open-sourced. |
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.
flip-link
is public so we can enable windows and mac right away
run: | | ||
cargo generate --git https://github.com/knurling-rs/app-template --branch main --name ${{ env.APPNAME }} | ||
cd ${{ env.APPNAME }} | ||
git apply ../.github/workflows/0001-configure-test-flip-link-app-for-nrf52840.patch |
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.
I was originally thinking of committing an initialized app-template instead of generating a new one on each CI. I think this approach (fetch a new one on each run) is better (at least as long as the app has git dependencies) because we'll get an error (the CI will fail) if the patch stops applying. Perhaps we should also set up a cron job to run CI on a daily basis to catch changes in app-template
opening as PR to check if the CI runs actually run.
still missing: app-template stuff
closes #5 once it's done