nuttxpr commented on PR #16039: URL: https://github.com/apache/nuttx/pull/16039#issuecomment-2746479405
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) Yes, this PR mostly meets the NuttX requirements, but it could be improved. Here's a breakdown: **Strengths:** * **Clear Summary:** The summary provides a good overview of the changes. * **Impact Assessment:** Addresses all the impact points, highlighting the addition of a new board and its effects on users and hardware. * **Testing Information:** Provides build host and target details, and includes build logs. **Weaknesses:** * **Missing "Before" Logs:** The "Testing logs before change" section is empty. This is crucial for demonstrating the actual change and its effectiveness. What was the behavior *before* this change? Did it not build? Did it build but not function correctly? Provide logs demonstrating the prior state. * **Insufficient "After" Logs:** The "After" logs only show a successful build and the `uname` command. This doesn't adequately demonstrate the functionality of the new board support. Show the output of the `leds` and `gpio` commands (or any other relevant commands) to verify they work as expected. * **Documentation:** While a separate PR for documentation is acceptable, the PR should at least mention *what* documentation will be added and where. This gives reviewers context. **Recommendations for Improvement:** 1. **Add "Before" logs:** Include logs demonstrating the build process or behavior before the changes were implemented. For a new board, this might show a build failure or the absence of the board configuration. 2. **Expand "After" logs:** Provide more comprehensive testing logs demonstrating the functionality of the `gpio` and `userleds` on the XIAO RA4M1. Ideally, include a simple test application that exercises these features and show its output. 3. **Clarify Documentation Plan:** Briefly state what documentation will be added in the separate PR (e.g., "Documentation for the XIAO RA4M1 board will be provided in a separate PR, including details on configuration and usage.") By addressing these points, the PR will be significantly stronger and easier for reviewers to evaluate. -- 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]
