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 #2758

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: apache/nuttx#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/nuttx#14376
@nuttxpr
Copy link

nuttxpr commented Oct 18, 2024

[Experimental Bot, please feedback here]

The provided PR description does not meet the NuttX requirements completely. Here's why:

Summary:

  • It lacks clarity on what constitutes a "Complex PR." Provide clear criteria for determining PR complexity.
  • It mentions a functional change ("CI Workflow shall run...") which is not directly related to the NuttX codebase. Rephrase this to highlight the benefit for NuttX development (e.g., "Reduce CI runtime for complex PRs...").

Impact:

  • The impact on build is described twice. Consolidate this information.
  • Be more specific about the impact on the user:
    • Will PR authors need to adapt to the change? How?
    • Will this affect PR review time?
  • Quantify the expected reduction in GitHub Runner usage.

Testing:

  • "See TODO" is not sufficient. Provide actual testing logs or links to specific CI runs that demonstrate the intended behavior.
  • Include tests for both simple and complex PRs across different architectures.
  • Explain how the criteria for a "Complex PR" were applied to the test cases.

Overall:

The PR description needs more details and concrete examples to demonstrate that the proposed changes are effective and meet the NuttX requirements. Clearly articulate the problem, the proposed solution, its impact, and how it has been tested.

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

@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 a799c3e into apache:master Oct 18, 2024
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[URGENT] Reducing our usage of GitHub Runners
3 participants