nuttxpr commented on PR #15968: URL: https://github.com/apache/nuttx/pull/15968#issuecomment-2712132579
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) The PR mostly meets the NuttX requirements but has some omissions and inaccuracies. Here's a breakdown: **Strengths:** * **Clear Summary:** The summary provides a good overview of the changes. * **Testing Confirmation:** Confirms testing and provides example output. (Although the "before" logs are missing.) * **User Impact Addressed:** Clearly states the impact on the user. **Weaknesses:** * **Inaccurate "Is new feature added?"**: Adding board support *is* a new feature. This should be YES. * **Missing "What functional part of the code is being changed?" in Summary:** Specify the affected directories/files (e.g., `boards/arm/rp2040/xiao-rp2040`). * **Missing "How does the change exactly work?" in Summary:** Briefly describe the implementation (e.g., "Adds board configuration files, pin definitions, serial driver instantiation, etc."). * **Missing Issue References (if applicable):** If this relates to any existing NuttX issues, they should be linked. * **Missing Before Logs:** Testing logs *before* the change are required to demonstrate the previous behavior. * **Incomplete Hardware Impact:** While stating "NO", the PR clearly adds support for a new board. This should be "YES" with a description (e.g., "Adds support for the Seeed Studio XIAO RP2350 board based on the RP2040 microcontroller."). Mentioning the architecture (arm) and the affected drivers (e.g., serial, GPIO, WS2812) is also good practice. * **Vague Documentation Impact:** "to be submitted on other PR" is insufficient. Either the documentation is included in this PR, or the linked PR that contains the documentation update should be referenced. * **Missing Build Host Details:** The testing section lacks details about the build host environment (OS, compiler version, etc.). * **Insufficient Target Details:** While `xiao-rp2350` is mentioned, specifying the configuration used (e.g., `xiao-rp2350:nsh`) would be more helpful. **Recommendations for Improvement:** 1. **Correct "Is new feature added?" to YES.** 2. **Complete the Summary section.** Provide details about the changed code sections and the implementation approach. 3. **Include "before" test logs.** Show the behavior before the change for comparison. 4. **Provide specific details about the build host and target environment.** 5. **Clarify Hardware Impact:** State YES and describe the added board support in detail. 6. **Address Documentation Impact:** Either include the documentation updates in the PR or provide a link to a separate documentation PR. 7. **Add Issue references if any.** By addressing these weaknesses, the PR will better adhere to the NuttX requirements and be easier for reviewers to assess. -- 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
