nuttxpr commented on PR #15427: URL: https://github.com/apache/nuttx/pull/15427#issuecomment-2572503467
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) No. The PR description lacks crucial details required by the NuttX guidelines. Here's why and what's missing: * **Summary:** While it mentions the "what," it lacks the "why" and "how." Why is this change necessary? Was there a bug or user request? How does the `quit` command now interact with the poweroff functionality (code-level explanation)? * **Impact:** The listed impacts are vague. The template asks for specific YES/NO answers followed by detailed explanations. For example: * **Impact on user:** YES, users can now power off the qemu-armv7a target using the `quit` command in NSH. * **Impact on build:** YES, the build process will now include poweroff functionality if `CNOFIG_BOARDCTL_POWEROFF` and `CONFIG_ARM_PSCI` are defined. * **Impact on hardware:** YES, This specifically impacts qemu-armv7a targets. Other architectures/boards are not affected. * **Impact on documentation:** Likely YES. Documentation should be updated to reflect this new feature and how to enable it. The PR should ideally include this documentation update. * **Security, Compatibility, Other:** These are all answered NO without explanation. Even if there's no impact, explicitly stating "NO" followed by a brief justification (e.g., "NO - No security implications are foreseen.") is better. * **Testing:** The testing information is insufficient. * **Build Host(s):** Missing operating system, CPU architecture, and compiler version used for building. * **Target(s):** More specific information is needed. "qemu-armv7a" is a start, but which specific QEMU version and machine type? Which NuttX configurations (nsh and knsh are mentioned, but full config names are preferable)? * **Logs:** "CI checks" isn't enough. The template explicitly asks for *logs before* and *after* the change demonstrating the change's effect. Even simple logs showing the system powering off after the `quit` command are necessary. In short, the PR needs significantly more detail to meet NuttX requirements. It needs to be more explicit about the change's motivation, implementation, and testing to allow for proper review and ensure code quality. -- 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]
