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: Skip half of RISC-V, Xtensa and Simulator targets when a Complex PR is created / updated #14400

Merged
merged 1 commit into from
Oct 18, 2024

Conversation

lupyuen
Copy link
Member

@lupyuen lupyuen commented Oct 18, 2024

Summary

When we submit or update a Complex PR that affects All Architectures (Arm, RISC-V, Xtensa, etc): CI Workflow shall run only half the jobs for RISC-V, Xtensa and Simulator:

  • risc-v-01 to 03
  • xtensa-01
  • sim-01

When the Complex PR is Merged: CI Workflow will still run all jobs for RISC-V, Xtensa and Simulator:

  • risc-v-01 to 06
  • xtensa-01 to 02
  • sim-01 to 02

Simple PRs with One Single Arch / Board will build the same way as before:

  • risc-v-01 to 06
  • xtensa-01 to 02
  • sim-01 to 02

We hope to lower drastically our usage of GitHub Runners before the ASF Deadline, as explained here: #14376

Impact

When we submit or update a Complex PR: CI Workflow shall run only half the jobs for RISC-V, Xtensa and Simulator:

  • risc-v-01 to 03
  • xtensa-01
  • sim-01

No changes to the CI Workflow when we merge a Complex PR. No impact for Simple PRs.

Testing

Creating a Simple PR for RISC-V will run jobs risc-v-01 to 06 (like before):

Creating a Complex PR will only run jobs:

Merging a Complex PR will run all jobs (like before):

…PR is created / updated

When we submit or update a Complex PR that affects All Architectures (Arm, RISC-V, Xtensa, etc): CI Workflow shall run only half the jobs for RISC-V, Xtensa and Simulator:
- `risc-v-01` to `03`
- `xtensa-01`
- `sim-01`

When the Complex PR is Merged: CI Workflow will still run all jobs for RISC-V, Xtensa and Simulator:
- `risc-v-01` to `06`
- `xtensa-01` to `02`
- `sim-01` to `02`

Simple PRs with One Single Arch / Board will build the same way as before:
- `risc-v-01` to `06`
- `xtensa-01` to `02`
- `sim-01` to `02`

We hope to lower drastically our usage of GitHub Runners before the ASF Deadline, as explained here: apache#14376
@github-actions github-actions bot added Area: CI Size: S The size of the change in this PR is small labels Oct 18, 2024
@lupyuen lupyuen linked an issue Oct 18, 2024 that may be closed by this pull request
@nuttxpr
Copy link

nuttxpr commented Oct 18, 2024

[Experimental Bot, please feedback here]

NuttX PR Requirements Review

Meets Requirements? Partially

Summary:

The PR description clearly outlines the need to reduce GitHub Runner usage and proposes a change to the CI workflow. However, it lacks specifics on the exact implementation and criteria for distinguishing "Complex" vs "Simple" PRs.

Strengths:

  • Clear Problem Statement: The description effectively explains the motivation for the change (reducing GitHub Runner usage).
  • Impact Clearly Articulated: The impact on CI workflows for different PR types is well-defined.
  • Testing Evidence Provided: Links to test runs demonstrating the intended behavior are included.

Areas for Improvement:

  • Vague Definition: The term "Complex PR" needs a precise definition. What criteria will trigger the reduced CI workflow?
  • Implementation Details Missing: The description lacks specifics on how the CI workflow will be modified to achieve the desired behavior. What scripts or configuration changes are involved?
  • Incomplete Testing:
    • "TODO" markers indicate missing testing for "Simple PR" and "Merging a Complex PR" scenarios. These tests are crucial for verifying the complete functionality.
    • Consider adding tests that intentionally trigger the "Complex PR" criteria to demonstrate its effectiveness.

Recommendations:

  1. Define "Complex PR": Clearly specify the criteria (e.g., number of files changed, specific directories affected) that will trigger the reduced CI workflow.
  2. Provide Implementation Details: Explain how the CI workflow will be modified. Include code snippets or configuration changes to illustrate the implementation.
  3. Complete Testing:
    • Replace "TODO" markers with actual test results.
    • Include tests that specifically target the "Complex PR" criteria.

Overall:

The PR addresses a valid concern and outlines a potential solution. However, it needs more detail regarding implementation and a more robust testing approach before it can be deemed fully compliant with the NuttX PR requirements.

@lupyuen lupyuen marked this pull request as ready for review October 18, 2024 21:49
Copy link
Contributor

@cederom cederom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @lupyuen :-) You can merge at 0000UTC to start metrics gathering :-)

@lupyuen lupyuen merged commit a94c099 into apache:master Oct 18, 2024
29 checks passed
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.

[URGENT] Reducing our usage of GitHub Runners
3 participants