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

{bp-15151} sched/clock: cleanup g_system_ticks reference if arch timer is enabled #15175

Merged
merged 2 commits into from
Dec 13, 2024

Conversation

jerpelea
Copy link
Contributor

Summary

sched/clock: cleanup g_system_ticks reference if arch timer is enabled

Included:
#15139

Impact

RELEASE

Testing

CI

@github-actions github-actions bot added Area: OS Components OS Components issues Size: S The size of the change in this PR is small labels Dec 13, 2024
@nuttxpr
Copy link

nuttxpr commented Dec 13, 2024

[Experimental Bot, please feedback here]

No, this PR does not fully meet the NuttX requirements as described. While it provides a summary and mentions testing with CI, it lacks crucial details.

Here's what's missing:

  • Summary: Lacks detail. Why is this cleanup necessary? What problem does it solve? How exactly does the removal of g_system_ticks work when the architecture timer is enabled? What is the functional change? The linked PR should be referenced in the "Related NuttX Issue" section, not embedded within the summary.

  • Impact: Simply stating "RELEASE" is insufficient. Each impact category needs to be addressed with "YES" or "NO" and a description if "YES". For example, even if there's no user impact, explicitly state "Impact on user: NO". Consider the implications of this change. Does it improve performance? Reduce memory usage? Does it affect any specific architectures, boards, or drivers? Does this change require documentation updates?

  • Testing: "CI" is not sufficient. While CI testing is important, local testing details are required. Provide the specific build host OS, CPU, compiler, and the target architecture, board, and configuration used for testing. Critically, the testing logs before and after the change are missing. These logs demonstrate the issue the PR addresses and confirm the fix. Just saying it "works as intended" isn't enough; show how it works as intended.

In short, the PR needs to be much more descriptive and provide concrete evidence of testing and impact analysis to meet the NuttX requirements.

@xiaoxiang781216 xiaoxiang781216 merged commit 6825f66 into apache:releases/12.8 Dec 13, 2024
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: OS Components OS Components issues 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.

5 participants