nuttxpr commented on PR #16150: URL: https://github.com/apache/nuttx/pull/16150#issuecomment-2782733166
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) No, this PR description does not fully meet the NuttX requirements. While it provides some information, it lacks crucial details in several sections. Here's a breakdown of what's missing: **Summary:** * **Missing "Why":** The summary states *what* was changed, but not *why*. Was this requested by a user? Does it fix a bug? Is it part of a larger feature implementation? * **Missing "How":** It mentions adding support, but doesn't explain *how* it works. Did it involve new drivers? Modifications to existing code? What specific functionality is enabled? **Impact:** * **Insufficient Detail:** Simply stating "new features" is not enough. The impact section requires more detailed answers. While the impact might be minimal for some areas, explicitly stating "NO" with a brief justification is better than leaving it blank. For example: * **User Impact:** YES. Users can now utilize the IWDG and WWDG features on the nucleo-c071rb board. They will need to consult updated documentation to understand configuration and usage. * **Build Impact:** Potentially YES. If new Kconfig options were added, the build configuration process might be affected. Specify any changes to dependencies or configuration options. * **Hardware Impact:** YES. This specifically affects the stm32f0l0g0 architecture and the nucleo-c071rb board. Mention any changes to pin assignments or resource usage. * **Documentation Impact:** YES. Documentation needs to be updated to describe the new watchdog features, their configuration, and usage examples. State whether this documentation update is included in the PR. * **Security Impact:** Potentially YES. Watchdogs are often security-related. Explain any potential security implications of this change, even if just to say "No impact on security beyond the intended watchdog functionality." * **Compatibility Impact:** Likely NO. But it's good to explicitly state this and give a brief reason (e.g., "No compatibility impact as this adds new features without modifying existing functionality."). **Testing:** * **Insufficient Host Detail:** What OS, CPU architecture, and compiler version were used for building? Be specific. (e.g., "Build Host: Linux x86_64, GCC 12.2") * **Insufficient Target Detail:** Be more specific about the target configurations. (e.g., "Target: stm32f0l0g0:nucleo-c071rb and stm32l072cz:b-l072z-lrwan1") * **Missing Logs:** The testing logs sections are empty. Provide actual log output demonstrating the watchdogs functioning correctly before and after the changes. If the logs are extensive, consider linking to a separate file or using a service like pastebin. **In short, the PR description needs to be significantly expanded to meet the NuttX requirements.** Provide the missing details and ensure all sections are thoroughly addressed, even if the answer is "NO." This will make the review process much smoother and increase the chances of your PR being accepted. -- 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
