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: Run CI Job other only for Simple x86 PR #14896

Merged
merged 1 commit into from
Nov 22, 2024

Conversation

lupyuen
Copy link
Member

@lupyuen lupyuen commented Nov 22, 2024

Summary

Presently, Simple x86 PRs will run All CI Checks (across all Architectures), as reported here: #14885 (comment)

This PR fixes the CI Build Rules, so that Simple x86 PRs will run only One Single CI Job: other.

Impact

When we create or update a Simple x86 PR: Only CI Job other shall be executed.

No impact on other PRs.

Testing

We tested the updated Build Rules arch.yml:

Regression Test:

Presently, Simple x86 PRs will run All CI Checks (across all Architectures), as reported here: apache#14885 (comment)

This PR fixes the CI Build Rules, so that Simple x86 PRs will run only One Single CI Job: `other`.
@nuttxpr
Copy link

nuttxpr commented Nov 22, 2024

[Experimental Bot, please feedback here]

Yes, this PR meets the NuttX requirements. The PR description clearly explains the problem (unnecessary CI runs for simple x86 PRs), the solution (updated CI build rules), and the impact (only the other CI job will run for simple x86 PRs, no impact on other PRs). The testing section provides ample evidence that the changes work as intended, including regression tests to ensure other architectures are not affected. The links to the test runs are particularly helpful.

While the provided information is comprehensive, consider adding these minor improvements to future PRs:

  • Specific file changes: Mentioning the specific file(s) modified (e.g., arch.yml) in the Summary or Impact section would be helpful.
  • "Before" and "After" logs: While links to CI runs are provided, including snippets of relevant log output directly in the "Testing logs before change" and "Testing logs after change" sections could offer a quicker understanding of the change's effect. Focus on the key differences. If the logs are very long, consider highlighting the relevant parts.
  • Explicitly answer all Impact questions: While the impact is clear, explicitly answering each impact question (even with "NO") improves readability and ensures all potential impacts are considered. For example:
    • Impact on user: NO
    • Impact on build: YES (CI build rules changed for x86 PRs)
    • ...and so on for all impact questions.

These minor additions will make the PR even clearer and easier to review.

@xiaoxiang781216 xiaoxiang781216 merged commit c0669e3 into apache:master Nov 22, 2024
27 checks passed
lupyuen added a commit to lupyuen2/wip-nuttx-apps that referenced this pull request Jan 8, 2025
This PR increases the CI Jobs for Complex PRs from 50% to 100%, as explained here:
- apache/nuttx#15451 (comment)

This PR also includes the fix for Simple x86 PR:
- apache/nuttx#14896
xiaoxiang781216 pushed a commit to apache/nuttx-apps that referenced this pull request Jan 8, 2025
This PR increases the CI Jobs for Complex PRs from 50% to 100%, as explained here:
- apache/nuttx#15451 (comment)

This PR also includes the fix for Simple x86 PR:
- apache/nuttx#14896
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.

3 participants