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

github: workflows: Remove continue-on-error #58

Merged

Conversation

jhedberg
Copy link
Collaborator

We want the workflows to fail for these steps, so remove continue-on-error.

We want the workflows to fail for these steps, so remove continue-on-error.

Signed-off-by: Johan Hedberg <[email protected]>
@jhedberg
Copy link
Collaborator Author

jhedberg commented Oct 29, 2024

Note: I'll wait until zephyrproject-rtos/zephyr#80537 has been merged, and then update west.yml in this PR, since otherwise our branch protection policy will prevent merging due to the failing checks.

@jerome-pouiller
Copy link
Contributor

The build fails on windows. Probably I shouldn't removed this snippet:

      if [ "${{ runner.os }}" = "Windows" ]; then
        EXTRA_TWISTER_FLAGS="--short-build-path -O/tmp/twister-out"
      fi

@jhedberg
Copy link
Collaborator Author

The build fails on windows. Probably I shouldn't removed this snippet:

      if [ "${{ runner.os }}" = "Windows" ]; then
        EXTRA_TWISTER_FLAGS="--short-build-path -O/tmp/twister-out"
      fi

Indeed. That said, do we think it really adds sufficient value to have macOS and Windows in the CI?. I would expect that upstream takes care of testing that the SDK works on all three platforms. That said, if we ever start introducing non-Zephyr SDK dependencies to our CI workflows (say, Commander or JLink), then I think it'd be important to check that the steps still work on all Zephyr SDK supported operating systems.

@jerome-pouiller
Copy link
Contributor

The failure on Windows is not introduced by this PR. So, I suggest to merge it.

This should fix the CI failues we've been having with `west twister`.

Signed-off-by: Johan Hedberg <[email protected]>
Upstream should do a good enough job of testing all supported Zephyr SDK OS
versions. We can reintroduce macOS and Windows if/when we start doing more
advanced things in CI that Zephyr upstream doesn't already do.

Signed-off-by: Johan Hedberg <[email protected]>
Instead of complicated grep + sed combinations, simply use the west list
command to get the current revision of the zephyr tree.

Signed-off-by: Johan Hedberg <[email protected]>
@jhedberg jhedberg force-pushed the workflow_continue_on_error branch from a922d22 to 558d7a1 Compare October 30, 2024 20:54
@jhedberg
Copy link
Collaborator Author

The failure on Windows is not introduced by this PR. So, I suggest to merge it.

I'd have to tweak the repo settings first, since right now it's set so that even an admin can't press the Merge button when there's a failed Check. I went with disabling Windows and macOS instead for now. We can re-enable them later if there's a clear justification for it.

Don't use "build" since it becomes difficult to distinguish this from the
actual build workflows when defining rule sets for the repo.

Signed-off-by: Johan Hedberg <[email protected]>
@jhedberg jhedberg merged commit 02e119b into SiliconLabsSoftware:main Oct 30, 2024
3 checks passed
@jhedberg jhedberg deleted the workflow_continue_on_error branch October 30, 2024 21:06
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.

3 participants