nuttxpr commented on PR #16188: URL: https://github.com/apache/nuttx/pull/16188#issuecomment-2796290003
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) Yes, this PR appears to meet the NuttX requirements, although some sections could be more detailed. While concise, it's missing specifics. Here's a breakdown of what's good and what could be improved: **Strengths:** * **Clear Summary:** The summary explains the "why," "what," and "how" of the change. * **Impact Marked as RELEASE:** This correctly flags the impact as potentially affecting a release. **Weaknesses:** * **Missing Issue References:** Even if there isn't a directly related issue, stating "N/A" or "None" is better than omitting these entirely. * **Impact Lacks Detail:** While marking it as RELEASE is good, the description is insufficient. Explain *how* this impacts a release. Does it fix a critical bug? Does it improve performance? This helps reviewers understand the severity. * **Testing is Too Brief:** "CI" is not enough. While CI is important, reviewers need to see *specific* test results. Which targets were tested? What commands were run? What was the expected output? Even if the CI passes, providing snippets of relevant logs here is crucial for demonstrating the fix and allowing reviewers to understand the change's effect. * **Missing Build Host Information:** The testing section should specify the build host OS, CPU, and compiler version used. * **Missing Target Details:** Be more specific than "CI." List the *exact* `arch:board:config` tested (e.g., `riscv:esp32c3-devkitm-1:nsh`). * **Missing "Before" and "After" Logs:** The template explicitly requests logs demonstrating the issue before the fix and the corrected behavior afterward. These are essential for verifying the fix. **Recommendation:** Expand the Testing and Impact sections significantly. Provide concrete examples and details to give reviewers confidence in the change. Even a small, focused log snippet is more helpful than just stating "CI." Don't rely solely on CI to convey the testing information. Make the PR self-contained and easy to review. -- 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
