nuttxpr commented on PR #15396: URL: https://github.com/apache/nuttx/pull/15396#issuecomment-2566360905
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) __Fill In The Commit Message:__ This PR contains a Commit with an Empty Commit Message. Please fill in the Commit Message with the PR Summary. Yes, this PR appears to meet the NuttX requirements, although some sections could be more explicit. **Strengths:** * **Clear Summary:** The summary explains the "why," "what," and "how" of the changes. The focus on `ARCH_RV_HARTID_BASE` is helpful. * **Impact Section Identifies Affected Area:** Calling out RISC-V SMP is good. * **Testing Mentions Environments:** Referencing GitHub CI and QEMU provides some testing context. **Weaknesses (and suggestions for improvement):** * **Missing Issue References:** If this relates to a specific issue, link it. * **Impact Section Needs More Detail:** Expand on *how* the changes impact RISC-V SMP. Address all the sub-points (user impact, build impact, hardware impact, documentation, security, compatibility). Even if the answer is "NO," explicitly state it. For example: * **Impact on user:** NO. Users should not need to adapt to this change as it is internal. * **Impact on build:** YES. The build system now uses `CONFIG_ARCH_RV_HARTID_BASE != 0` as a condition for compiling `riscv_cpuindex.c`. * ... (and so on for all impact categories) * **Testing Logs Missing:** The template specifically requests "Testing logs before change" and "Testing logs after change." Include relevant log snippets or explain why they aren't applicable. If using CI, link to a successful run. Be specific about what was tested (e.g., "Boot test on QEMU with 2 cores," "SMP scheduler test on QEMU"). **Example for the Testing Section:** ``` Testing: Build Host: Linux, x86_64, GCC 12.2 Target: QEMU, RISC-V rv64_imafdc_zicsr_zifencei, 2 cores GitHub CI Run: [Link to CI Run] Testing logs before change (demonstrating issue X): ``` <Relevant log snippet showing the problem this PR fixes> ``` Testing logs after change (demonstrating the fix): ``` <Relevant log snippet showing the corrected behavior> ``` Tests Performed: * Boot test on QEMU with 2 cores - successful. * SMP scheduler test on QEMU - successful. ``` By addressing these weaknesses, the PR will be even 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: commits-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
