nuttxpr commented on PR #2877: URL: https://github.com/apache/nuttx-apps/pull/2877#issuecomment-2517418978
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) Yes, this PR appears to meet the basic NuttX requirements, although it's missing some details. While concise, it could be improved by providing more specifics in the Impact and Testing sections. For example: * **Impact:** While it says "case only," it doesn't explicitly answer all the impact questions (e.g., Impact on user, build, hardware, documentation, security, compatibility). Even if the answer is "NO" for most, stating that explicitly is helpful for reviewers. The "fix cmake support" comment should be elaborated on – what exactly was fixed and why? * **Testing:** "CI-test, local qemu mps2" is a good start, but more detail would be beneficial. What specific CI tests were run? What configuration was used for qemu-mps2? While the template asks for "Testing logs before change" and "Testing logs after change," providing the full logs is usually not necessary. Instead, summarize the key differences or observed improvements. For example, "Before: Assertion failed. After: Test passed." While the summary is brief, it does address the core questions. The provided information suggests that the changes are limited in scope (a new test case and a CMake fix), which simplifies the review process. However, providing explicit answers to all the questions, even if they are negative ("NO"), strengthens the PR and makes it easier for reviewers to quickly assess the changes. -- 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
