Skip to content

Conversation

@lupyuen
Copy link
Member

@lupyuen lupyuen commented Oct 2, 2024

Summary

This PR proposes to enhance the CI Workflow, to skip the unnecessary NuttX Builds. Currently, NuttX Devs wait for the CI Builds to complete across All Architectures (Arm32, Arm64, RISC-V, Xtensa), even though they modified a Single Architecture. With this PR, the CI Workflow will build only the Modified Architecture.

The solution uses the Arch Labels for PRs. We target only the Simple PRs: One Arch Label + One Size Label (e.g. "Arch: risc-v, Size: XS")

  • If "Arch: arm / arm64" is the only non-size label, then we build other, arm-01, arm-02, ...
  • If "Arch: risc-v" is the only non-size label, then build risc-v-01, risc-v-02
  • If "Arch: xtensa" is the only non-size label, then build xtensa-01, xtensa-02
  • The above rules apply when the PR is Created or Modified
  • When the PR is Merged: All targets shall be built

The code is explained here: #13775

Impact

For Simple PRs: CI Builds shall be skipped for the targets that don't match the Arch Label

For Complex PRs: No change, all targets shall be built

Testing

When we create a PR, the irrelevant builds are skipped in seconds. Click "Show All Jobs" to reveal the timings: (Merge PR will compile all targets, same for Others)

This PR proposes to enhance the CI Workflow, to skip the unnecessary NuttX Builds. Currently, NuttX Devs wait for the CI Builds to complete across All Architectures (Arm32, Arm64, RISC-V, Xtensa), even though they modified a Single Architecture. With this PR, the CI Workflow will build only the Modified Architecture.

The solution uses the Arch Labels for PRs. We target only the Simple PRs: One Arch Label + One Size Label (e.g. "Arch: risc-v, Size: XS")
- If "Arch: arm / arm64" is the only non-size label, then we build `other`, `arm-01`, `arm-02`, ...
- If "Arch: risc-v" is the only non-size label, then build `risc-v-01`, `risc-v-02`
- If "Arch: xtensa" is the only non-size label, then build `xtensa-01`, `xtensa-02`
- The above rules apply when the PR is Created or Modified
- When the PR is Merged: All targets shall be built

The code is explained here: apache#13775
@github-actions github-actions bot added Area: CI Size: S The size of the change in this PR is small labels Oct 2, 2024
@nuttxpr
Copy link

nuttxpr commented Oct 2, 2024

[Experimental Bot, please feedback here]

The provided PR description mostly meets the NuttX requirements, but could use a few minor improvements:

Strengths:

  • Summary: Clearly explains the problem, proposed solution, and how it works. Linking to the issue is helpful.
  • Impact: Addresses all the points, highlighting the key benefit for simple PRs and maintaining existing behavior for complex ones.
  • Testing: Provides multiple examples with links, demonstrating the intended behavior in various scenarios. Clearly distinguishes between PR creation and merging.

Areas for Improvement:

  • Testing:
    • Before Change Logs: While you mention "the irrelevant builds are skipped in seconds," providing actual "before" logs, even if they just show the numerous builds being triggered, would offer a direct comparison point.
    • Build Host/Target Details: Specify the host OS, compiler, specific target boards, and configurations used for testing. This adds credibility and helps reviewers reproduce your results.

Conciseness:

  • The description is already quite concise. You could potentially shorten the "Testing" section by summarizing the results instead of listing each architecture separately, especially since the links provide the details. For example:

    "Testing confirms that irrelevant builds are skipped in seconds when creating a PR with a single architecture label. This has been verified across ARM32, ARM64, RISC-V, and Xtensa architectures (see links for detailed logs). Merge PRs and PRs without clear architecture labels continue to build all targets as expected."

Overall:

This PR description effectively conveys the necessary information. Adding the "before" logs and some specific build details would further strengthen it.

@xiaoxiang781216
Copy link
Contributor

should we optimize sim arch too?

@xiaoxiang781216 xiaoxiang781216 linked an issue Oct 3, 2024 that may be closed by this pull request
1 task
@lupyuen
Copy link
Member Author

lupyuen commented Oct 3, 2024

should we optimize sim arch too?

@xiaoxiang781216 Yep I'll do this in the next PR. Right now I'm keen to monitor the effect of the updated CI Workflow, to make sure that we comply with the ASF Policy for GitHub Actions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: CI Size: S The size of the change in this PR is small

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEATURE] Enhance the CI Workflow to skip the Unmodified Architectures

3 participants