nuttxpr commented on PR #16228: URL: https://github.com/apache/nuttx/pull/16228#issuecomment-2809353486
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) This PR appears to meet the NuttX requirements, although some minor improvements could be made. **Strengths:** * **Clear Summary:** The summary clearly outlines the changes and their purpose. * **Detailed Impact Assessment:** The impact section addresses all the required points and provides specific details about the changes for ESP32S3. * **Thorough Testing:** The testing section describes the test setup, procedure, and results in detail, including example output. The mention of a separate automated test application is also a positive. **Areas for Improvement:** * **Conciseness:** The testing section could be more concise. While detail is good, the extensive log output could be trimmed or moved to a separate file referenced in the PR. Summarizing the results across all boards is helpful, but the full tables aren't necessary in the PR description itself. Offer to provide the detailed data upon request. * **Testing Logs Before Change:** While you mention a testing procedure, you didn't include "before change" logs. These logs are essential to demonstrate the actual improvement or fix provided by the PR. Include a brief snippet of the output before the ADC changes were implemented, even if it simply shows a lack of functionality. * **Related Issues:** You mention updating the documentation, but it would be helpful to link any related NuttX or NuttX Apps issues that prompted this change, or that this change resolves. Even if there wasn't a specific issue, mentioning it ("No related issues") is good practice. **Verdict:** Overall, the PR seems well-structured and provides enough information to understand the changes. Addressing the minor points above, particularly including "before change" logs, would further strengthen the PR. -- 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
