nuttxpr commented on PR #13561: URL: https://github.com/apache/nuttx/pull/13561#issuecomment-2363740323
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) The PR Summary and Testing sections appear to meet NuttX requirements, but the Impact section needs more detail. Here's a breakdown and suggestions for improvement: **Good:** * **Summary:** Clearly explains the "why", "what" and "how" of the change. The size reduction is a good quantitative measure of the impact. * **Testing:** Specifies the testing environment ("ostest") and provides evidence of testing. Ideally, the "before" and "after" logs would highlight the specific area improved by this change (e.g., context switch timings). **Needs Improvement:** * **Impact:** Simply stating "riscv" is insufficient. You need to elaborate on *how* this change impacts RISC-V: * **Specific architectures/boards affected:** Does this impact all RISC-V targets or just specific ones? * **User impact:** Will users experience any noticeable differences? Faster context switching might be an improvement worth mentioning. * **Build impact:** While the code size reduction is noted, are there any changes to build configuration or dependencies? * **Documentation:** Does any documentation need updating to reflect this change in `g_current_regs` behavior? **Example of Improved Impact Section** ``` ## Impact * **Is new feature added? Is existing feature changed?** Existing feature (interrupt handling) optimized. * **Impact on user:** Potentially improved performance due to faster interrupt context switching on RISC-V targets. This impact may be most noticeable in applications with frequent interrupts. * **Impact on build:** Code size reduction of 322 bytes on RISC-V targets. No changes to build process or dependencies. * **Impact on hardware:** Affects all RISC-V architectures supported by NuttX. No impact on specific boards or drivers. * **Impact on documentation:** The behavior of `g_current_regs` on RISC-V has changed, so documentation may require updates to reflect its reduced functionality. * **Impact on security, compatibility:** None foreseen. ``` By providing specific details in the Impact section, you make it easier for reviewers to assess the risks and benefits of your change and ensure it aligns with NuttX standards. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
