nuttxpr commented on PR #16103: URL: https://github.com/apache/nuttx/pull/16103#issuecomment-2769824652
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) Yes, this PR likely meets the basic NuttX requirements, but falls short in the "Testing" section. While it links to a mailing list discussion and identifies the problem and solution, the provided testing information is insufficient. Specifically: * **Missing concrete test results:** "CI" is not enough. The PR needs to show *specific* before-and-after log output demonstrating the incorrect behavior and its correction. What function calls were made? What were the incorrect return values? What are the corrected return values? * **Lack of platform details:** While the summary mentions AVR, the testing section should explicitly list the *specific* AVR hardware or simulator used for testing. This includes the board, configuration, and compiler version. If tested on other architectures, that should be noted too. * **Impact under-specified:** While mentioning the need for review is good, the Impact section needs more details. "Accidentally break something" is too vague. Does this change affect any drivers or applications relying on timing functions? Are there potential performance impacts from using `long` instead of `int`? Therefore, while the PR addresses a real issue and proposes a solution, it needs significant improvements in documenting the testing and impact before being considered ready for merge. -- 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
