nuttxpr commented on PR #15055: URL: https://github.com/apache/nuttx/pull/15055#issuecomment-2519150738
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) No. This PR summary is insufficient. It lacks crucial information required by the NuttX guidelines. Here's why and what's missing: * **Insufficient Summary:** "xtensa: inline up_switch_context" is a title, not a summary. It states *what* was changed but not *why*. The summary needs to explain the rationale behind inlining `up_switch_context`. What problem does it solve? Is it a performance optimization? Does it fix a bug? What part of the code is affected? How does inlining `up_switch_context` actually work and achieve the desired effect? * **Missing Issue References:** Are there any related NuttX or NuttX Apps issues that prompted this change? If so, those need to be linked. * **Incomplete Impact Assessment:** While "xtensa" suggests the impact is limited to that architecture, it's not detailed enough. The guidelines require "YES/NO" answers for each impact category, followed by descriptions if the answer is "YES". Even if the answer is "NO" for most, it should be explicitly stated. For example: * Impact on user: NO * Impact on build: NO (or YES if the build process is altered for xtensa) * Impact on hardware: YES (Affects xtensa architecture. Specifically, ... describe which xtensa chips/boards are affected) * Impact on documentation: NO (or YES if documentation needs updating) * Impact on security: NO (or YES, and explain why) * Impact on compatibility: NO (or YES, and specify what kind of compatibility is affected) * **Inadequate Testing Information:** "ci ostest" and "esp32s3-devkit:smp" are insufficient. The PR needs to specify the *host* operating system, CPU architecture, and compiler used for the build. It also needs to clearly identify the *target* architecture, board, and configuration. Crucially, it's missing the *actual testing logs* before and after the change, which are mandatory. Simply stating what tests were run doesn't prove the change works correctly or demonstrate the benefits (if any). In short, the PR needs significant revision to meet the NuttX requirements. It needs to provide more context, detail, and evidence to justify the change and demonstrate its correctness. -- 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]
