nuttxpr commented on PR #15850: URL: https://github.com/apache/nuttx/pull/15850#issuecomment-2663207498
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) Yes, this PR appears to mostly meet the NuttX requirements, but it is missing some crucial information. Here's a breakdown of what's missing and how to improve it: * **Summary:** While it explains *what* the change does, it lacks the *why*. Is this addressing a specific issue users were facing? Does it improve performance or robustness? A more compelling summary would explain the motivation behind the change. Also, link any related NuttX issues. * **Impact:** While it mentions the user impact, it needs more detail. Specifically: * **Impact on user (YES):** How will users enable this new configuration option? Will they need to modify their device tree or configuration files? Provide clear instructions. * **Impact on build (Possibly YES):** Does this introduce any new Kconfig options? If so, describe them. * **Impact on hardware (YES):** This explicitly affects the SAMv7. State this clearly. Are other architectures impacted at all (even indirectly)? * **Impact on documentation (YES):** Documentation *must* be updated to reflect this new configuration option. Mention that this update is included in the PR or will be provided in a separate PR. * **Impact on security, compatibility:** While likely "NO" in this case, explicitly stating "NO" with a brief justification (e.g., "NO - This change only affects PWM configuration and does not introduce any new security vulnerabilities.") is better than leaving them blank. * **Testing:** This section is completely empty. Provide details about your testing environment and *actual* logs demonstrating the change's behavior before and after. This is critical for reviewers to assess the correctness and stability of the change. Include the specific commands used for testing. **In short:** The PR needs more context, details on user impact and build changes, confirmation of documentation updates, explicit "NO" answers with brief justifications where applicable, and, most importantly, actual testing logs and procedures. -- 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]
