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

CI: continue BSP building workflow if build any BSP fails #745

Merged
merged 4 commits into from
Aug 20, 2024

Conversation

ianrrees
Copy link
Contributor

Summary

Potential fix for #744 - this change to CI has the "Build BSPs" workflow continue regardless of tier >1 BSP builds failing.

One downside to this approach is that the overall workflow run may show success despite there actually being BSP build failures. This PR is a test case to see how obvious this situation is in practice - a tier 2 BSP is expected to fail.

@ianrrees ianrrees force-pushed the ci-continue-if-tier2-fails branch 4 times, most recently from d3877d0 to 73fb006 Compare August 18, 2024 05:10
This is a step towards allowing the job to continue if any tier >1 BSPs
fail to build.
Resolves a warning about Node deprecation
@ianrrees ianrrees force-pushed the ci-continue-if-tier2-fails branch from 73fb006 to cf4ef86 Compare August 18, 2024 05:11
@ianrrees
Copy link
Contributor Author

ianrrees commented Aug 18, 2024

The required statuses come from the project's branch protection settings, they would need to be updated after this change is accepted in to master. The list of checks for Tier 1 BSPs would be replaced with a single check on "Tier 1 BSPs all built on stable".

@ianrrees ianrrees changed the title CI: Continue BSP building workflow if build of tier >1 BSP fails Draft: CI continue BSP building workflow if build of tier >1 BSP fails Aug 18, 2024
@ianrrees ianrrees force-pushed the ci-continue-if-tier2-fails branch 2 times, most recently from b75f83a to 86a4855 Compare August 19, 2024 06:21
@ianrrees ianrrees changed the title Draft: CI continue BSP building workflow if build of tier >1 BSP fails CI: continue BSP building workflow if build any BSP fails Aug 19, 2024
@jbeaurivage
Copy link
Contributor

I wouldn't say that a Tier 2 is necessarily expected to fail. As discussed on Matrix, Tier 2 BSPs are locked to a specific crates.io HAL version, so they normally should succeed when nothing has been changed (unless a dependency breaks semver, like is the case for the itsybitsy_m0.

I don't know how much complexity this would add, but just a suggestion to get around this:

Would it be feasible to only run CI on BSPs if:

  • It is a Tier 1 BSP
  • If it's a Tier 2 BSP, run CI only if the BSP had some files changed in the PR

@ianrrees
Copy link
Contributor Author

I wouldn't say that a Tier 2 is necessarily expected to fail

Sorry, that was poor wording on my part, meant that in this specific PR the itsybitsy m0 is expected to fail.

Would it be feasible to only run CI on BSPs if...

As external issues arise, with this change we'll know about them but they won't interfere with work. To me, this seems a bit better than letting issues silently accumulate, or relying on users to report them.

That said, I think that would be possible, and likely worth doing if we start running in to issues with time/cost of building all the BSPs.

@ianrrees ianrrees force-pushed the ci-continue-if-tier2-fails branch from 86a4855 to b8ee766 Compare August 19, 2024 23:27
@jbeaurivage jbeaurivage merged commit bd5c82f into atsamd-rs:master Aug 20, 2024
88 of 108 checks passed
ianrrees added a commit to ianrrees/atsamd that referenced this pull request Aug 21, 2024
)

* CI: Use BSP objects instead of names in job matrix

This is a step towards allowing the job to continue if any tier >1 BSPs
fail to build.

* CI: Continue if job fails for tier >1 BSP build

* CI: Update to newer checkout action

Resolves a warning about Node deprecation

* CI: Add job to show Tier 1 BSPs built on stable
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