Skip to content
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 all the targets #35

Merged
merged 24 commits into from
Jul 20, 2022
Merged

Add CI to build all the targets #35

merged 24 commits into from
Jul 20, 2022

Conversation

SergioGasquez
Copy link
Member

Expanded the CI to build the project for all the targets using the Dockerfile generated for devcontainer support.

@@ -63,8 +63,11 @@ jobs:
board: ['esp32', 'esp32s2', 'esp32s3', 'esp32c3']
std: ['true', 'false']
steps:
- name: Setup | cargo-generate
run: cargo install cargo-generate
- name: Install cargo-generate
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why this and not just cargo install cargo-generate?

Copy link
Member Author

Choose a reason for hiding this comment

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

I changed to the binary approach as it's way faster. If you think is better to do it with cargo install I am happy to undo this change

Copy link
Collaborator

@ivmarkov ivmarkov left a comment

Choose a reason for hiding this comment

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

I'm not sure why we are now installing cargo-generate that way?

@SergioGasquez
Copy link
Member Author

I'm not sure why we are now installing cargo-generate that way?

It's only because it's faster, but I wouldn't mind using cargo install if you think is better

@SergioGasquez
Copy link
Member Author

Installing cargo-generate from release binaries can also make the CI more "consistent" as it avoids some "external" errors on CI. Eg: Todays CI run failed because cargo install cargo-generate was failing

@ivmarkov
Copy link
Collaborator

@SergioGasquez Sorry for following up late, can you resolve the conflicts, then I'll merge?

@SergioGasquez
Copy link
Member Author

@SergioGasquez Sorry for following up late, can you resolve the conflicts, then I'll merge?

Hello @ivmarkov, do not worry about the delays! Conflicts are now resolved. Thanks for all your dedication!

@ivmarkov
Copy link
Collaborator

@SergioGasquez I think we need to temporarily disable the PlatformIO based build, until we figure out what is wrong there.

@SergioGasquez
Copy link
Member Author

@SergioGasquez I think we need to temporarily disable the PlatformIO based build, until we figure out what is wrong there.

I've commented those steps!

@ivmarkov
Copy link
Collaborator

@SergioGasquez BTW it still complains that the branch is unmergeable due to conflicts.
I think you need to rebase it on top of latest master first.

@SergioGasquez
Copy link
Member Author

It now shows that it can be merged with no conflicts! :)

@ivmarkov
Copy link
Collaborator

It can be merged but it cannot be rebased. Anyway, ill not use rebase then.

@ivmarkov ivmarkov merged commit 17bf14e into esp-rs:master Jul 20, 2022
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