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: Split the targets in sim-01 and add sim-03 #14428

Merged
merged 1 commit into from
Oct 22, 2024

Conversation

lupyuen
Copy link
Member

@lupyuen lupyuen commented Oct 21, 2024

Summary

CI Build Job sim-01 has become the bottleneck for compiling Complex PRs. This PR splits sim-01 and adds sim-03.

Before the Split: Simulator Jobs take up to 1.5 hours to complete

  • sim-01 (1 hour 31 mins): adb, citest, lvgl, matter
  • sim-02 (28 mins): posix_test, sqlite

After the Split: Simulator Jobs will complete within 1 hour

  • sim-01 (58 mins): adb, citest
  • sim-02 (35 mins): lvgl, matter
  • sim-03 (28 mins): posix_test, sqlite

This will help us comply with the ASF Policy for GitHub Actions, as explained here: #14376

Impact

When we create or modify a Complex PR, the CI Checks for Simulator will complete earlier. CI Job sim-01 will no longer be the bottleneck for compiling Complex PRs.

Testing

Timings Before the Split: https://github.com/nuttxpr/nuttx/actions/runs/11430701907

  • sim-01: 1 hour 31 mins
  • sim-02: 28 mins

Timings After the Split: https://github.com/lupyuen5/label-nuttx/actions/runs/11435289336

  • sim-01: 58 mins
  • sim-02: 35 mins
  • sim-03: 28 mins

This PR splits the CI Build Job sim-01 and adds sim-03:

Before the Split: Simulator Jobs take up to 1.5 hours to complete
- sim-01 (1 hour 31 mins): adb, citest, lvgl, matter
- sim-02 (28 mins): posix_test, sqlite

After the Split: Simulator Jobs will complete within 1 hour
- sim-01 (58 mins): adb, citest
- sim-02 (35 mins): lvgl, matter
- sim-03 (28 mins): posix_test, sqlite

This will help us comply with the ASF Policy for GitHub Actions, as explained here: apache#14376
@github-actions github-actions bot added Area: Tooling Area: CI Size: M The size of the change in this PR is medium labels Oct 21, 2024
@nuttxpr
Copy link

nuttxpr commented Oct 21, 2024

[Experimental Bot, please feedback here]

The PR generally meets the NuttX requirements, providing a clear explanation of the changes, impact, and testing. Here's a breakdown with suggestions for improvement:

Strengths:

  • Clear Summary: The summary effectively conveys the "why," "what," and "how" of the changes. Linking the related issue ([URGENT] Reducing our usage of GitHub Runners #14376) provides valuable context.
  • Impact Assessment: The impact section addresses relevant aspects, particularly the improvement in CI build times.
  • Testing Evidence: Providing links to actual CI runs with timings effectively demonstrates the impact of the changes.

Areas for Improvement:

  • Explicitly Address All Impact Points: While the current impact assessment covers the key aspects, it's beneficial to explicitly mention "NO" for areas where the changes have no impact (e.g., no user-facing changes, no hardware changes, no security implications). This ensures completeness and clarity.
  • Documentation Impact: Even if there are no documentation changes, explicitly stating "NO - No documentation updates are required" makes it clear that this aspect was considered.
  • Testing Logs: The requirement mentions including testing logs. While linking to CI runs is helpful, consider including snippets of relevant logs within the PR description to highlight any key changes or improvements.
  • Target Specificity: Under "Testing," specify the exact architectures and boards used for testing (e.g., "sim-qemu-arm").

Concise Feedback for the PR:

This PR effectively addresses the CI build time issue by splitting the sim-01 job. The provided testing evidence clearly demonstrates the improvements.

To further enhance this PR, please consider:

  • Explicitly addressing all impact points, even those with no change.
  • Confirming whether documentation updates are needed.
  • Including relevant snippets of testing logs in the description.
  • Specifying the architectures and boards used for testing.

lupyuen added a commit to lupyuen2/wip-nuttx-apps that referenced this pull request Oct 21, 2024
This PR syncs the Simulator Targets `sim-01`, `sim-02`, `sim-03` from `nuttx` repo to `nuttx-apps`: apache/nuttx#14428
@lupyuen lupyuen marked this pull request as ready for review October 21, 2024 21:12
xiaoxiang781216 pushed a commit to apache/nuttx-apps that referenced this pull request Oct 22, 2024
This PR syncs the Simulator Targets `sim-01`, `sim-02`, `sim-03` from `nuttx` repo to `nuttx-apps`: apache/nuttx#14428
@xiaoxiang781216 xiaoxiang781216 merged commit c4d754f into apache:master Oct 22, 2024
26 of 31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: CI Area: Tooling Size: M The size of the change in this PR is medium
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants