Check all option combinations in CI #26
Conversation
| let result = result | ||
| .into_iter() | ||
| .filter(|opts| { | ||
| !opts.contains(&"wifi".to_string()) || !opts.contains(&"ble".to_string()) |
There was a problem hiding this comment.
What's the purpose of this filter?
There was a problem hiding this comment.
Currently, when using ble and wifi, we throw an error: https://github.com/esp-rs/esp-generate/blob/main/src/main.rs#L408.
We should decide if we want to close the issue, and keep it like this. Or add support to allow using both options at the same time.
There was a problem hiding this comment.
We can fix this later, but we should definitely fix it as they aren't really mutually exclusive.
Probably worth fixing after lands: esp-rs/esp-hal#2301 as it should simplify initializing things
There was a problem hiding this comment.
We can fix this later, but we should definitely fix it as they aren't really mutually exclusive.
FYI, the TUI does not allow selecting both, this was a temporary fix to resolve #19. See this comment.
There was a problem hiding this comment.
Thanks
For the future I have two ideas
- define the options somewhere else so the xtask and main could use the same source of truth since currently we need to duplicate options (and also the xtask doesn't take those optional things into account currently)
- maybe we could have a way to allow generating into an existing directly (but only in CI with some magic ENV-var or something) and just remove the generated files between check runs and speed up checking by not having
cargo checkcheck a dependency if the features etc. didn change? (Not sure how feasible that is)
--all-combinationsflag to thecheckcommandCI test run: https://github.com/SergioGasquez/esp-generate/actions/runs/11570938398