nuttxpr commented on PR #15151: URL: https://github.com/apache/nuttx/pull/15151#issuecomment-2537994209
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) No, this PR does not fully meet the NuttX requirements. While it provides a summary and testing information, it lacks crucial details. Here's a breakdown of what's missing: * **Summary:** While it states the *what*, it omits the *why* and *how*. It needs to explain why cleaning up `g_system_ticks` is necessary when the architecture timer is enabled. What problem does it solve? How does the cleanup work? What code is changed? * **Impact:** Simply stating "N/A" is insufficient. Even seemingly minor changes can have unforeseen impacts. Each impact category needs to be explicitly addressed with "NO" or "YES" and a justification. For example, if there's truly no impact on the build, it should say "Impact on build: NO - No changes to the build process." This forces the submitter to consider each potential impact area. * **Testing:** "ci-check" is not sufficient. While CI is important, the PR needs to demonstrate *local* testing. The requirements specifically ask for: * Details about the build host and target(s) used for testing. * Logs from *before* the change. * Logs from *after* the change, showing the improvement or fix. Simply saying "ci-check" doesn't provide evidence of the change's effectiveness. In short, the PR needs more detail and explicit answers to the requirements to be considered complete. The current state makes it difficult to review and understand the full implications of the change. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
