nuttxpr commented on PR #15894: URL: https://github.com/apache/nuttx/pull/15894#issuecomment-2676441776
[**\[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 good starting point, it lacks crucial details. **Missing/Insufficient Information:** * **Summary:** While it mentions the "what," it's missing a strong "why." Why is this change *necessary*? What specific problem does it solve beyond a general desire for extensibility? A related NuttX issue would greatly strengthen this section. * **Impact:** The impact section is too vague. * **User Impact:** While it mentions custom boards, it doesn't specify *how* users would adapt. Would they need to change configurations? Write new drivers? Clearer examples are needed. * **Build Impact:** Does adding `weak_function` change the build process in any way? Even if the answer is "NO," explicitly state it. * **Hardware Impact:** Specify the affected architecture (ARM) and board (Raspberry Pi Pico, and any others). "Custom board" is not specific enough. List the *specific* hardware changes if any. * **Documentation Impact:** Does this require documentation updates? If so, have they been included in the PR? * **Security Impact:** Explicitly state "NO" if there's no security impact. * **Compatibility Impact:** Explicitly state "NO" or explain potential backward compatibility issues. Consider the impact on existing boards using the RP2040 SPI. * **Testing:** * **Build Host(s):** Be specific about the Linux distribution used, the compiler version (e.g., GCC 12.2.0), and any relevant build tools. * **Target(s):** Be precise about the target configurations tested. "raspberrypi-pico:nsh config" isn't precise enough. Specify the full configuration name (e.g., `configs/raspberrypi-pico/nsh`). For the custom board, provide details about its configuration. * **Logs:** The "Testing logs" sections are empty. Include relevant log output demonstrating the functionality before and after the change. Show how the new weak functions are being overridden and what the results are. Demonstrate that existing functionality is not broken. **Recommendations:** 1. **Clarify the "why":** Explain the specific problem this solves. Provide a concrete example of how the lack of this feature hindered development before. Link a related NuttX issue if one exists. 2. **Detailed Impact Assessment:** Be explicit about all impact categories, even if the answer is "NO." Provide specific details and examples where necessary. 3. **Thorough Testing Information:** Provide complete details about the build host and target environments. Include relevant log output demonstrating the change's functionality and the absence of regressions. Test and document the behavior when the weak functions are *not* overridden to ensure backward compatibility. 4. **Compiler Compatibility:** Research and address the compiler compatibility concerns regarding `weak_function`. Document any limitations or required compiler versions. By addressing these points, the PR will better meet the NuttX requirements and be more likely to be 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
