nuttxpr commented on PR #16299: URL: https://github.com/apache/nuttx/pull/16299#issuecomment-2846465365
[**\[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 of the changes and testing logs, it lacks crucial information required for proper review and integration. Here's a breakdown of the missing information: * **Summary:** While the summary explains *what* was changed, it doesn't explain *why*. Was there a bug report? Is this a new feature request? It needs more context. It also lacks links to any related issues. * **Impact:** The provided impact statement is far too brief. It needs to explicitly address *all* the listed impact points with "NO" or "YES" and provide descriptions where necessary. Even if the answer is "NO," it's helpful to explicitly state it for each point. For example: * Impact on user: NO * Impact on build: YES (TICK block configuration needs to be enabled) * Impact on hardware: YES (Specifically affects RP2350) * Impact on documentation: YES (Documentation should be updated to reflect the changes to the watchdog and TICK block configuration) * Impact on security: NO * Impact on compatibility: Potentially YES if other boards use a similar watchdog setup; this needs investigation. * **Testing:** While testing logs are provided, there's no information about the *Build Host* or *Target* as requested. Specify the OS, CPU, Compiler used for building, and the target architecture and board configuration. Also, the logs themselves are not very descriptive. While they show the watchdog triggering, they don't show how the TICK block is verified to be functioning correctly. More comprehensive tests are needed. Before/After logs aren't particularly useful in this case; focusing on the positive verification of the fix is more important. **Example of a better Impact section:** * Impact on user: NO (Users should not experience any functional changes besides a working watchdog timer) * Impact on build: YES (The `RP2350_TICK` config option must be enabled in the board configuration file. Example: `CONFIG_RP2350_TICK=y`) * Impact on hardware: YES (This change specifically addresses the RP2350 watchdog timer, which differs from the RP2040 implementation. It utilizes the system TICK blocks.) * Impact on documentation: YES (Documentation should be updated to explain the RP2350 watchdog timer configuration and the requirement for enabling the TICK blocks.) * Impact on security: NO (This change does not have any direct security implications.) * Impact on compatibility: YES (This change is specific to the RP2350. It needs to be verified that this doesn't negatively impact other boards that might share similar hardware blocks. A review of other board configurations is recommended.) By providing more complete information in these sections, the PR will be much easier to review, understand, and ultimately merge. -- 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: commits-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
