nuttxpr commented on PR #15160: URL: https://github.com/apache/nuttx/pull/15160#issuecomment-2538772727
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) No, this PR does not fully meet the NuttX requirements. Here's why: * **Summary:** While it explains the "why," it lacks clarity on the "what" and "how." Specifically, which file(s) were modified to increase the stack size? What was the original stack size? The summary should explicitly state "Increased the initial stack size from X to 8KB in [file name(s)]." * **Impact:** The impact section is far too brief. It only mentions `esp32s3` which is insufficient. Each impact category (user, build, hardware, documentation, security, compatibility) needs a YES/NO answer followed by a description if the answer is YES. In this case, since it affects hardware (esp32s3), the hardware impact should be "YES - Increases the initial stack size for all esp32s3 configurations." The other impact categories should also be explicitly addressed. For example: * Impact on user: NO * Impact on build: NO * Impact on hardware: YES - Increases the initial stack size for all esp32s3 configurations. * Impact on documentation: NO (or YES if documentation needs updating). * Impact on security: Potentially YES - Increased stack size could mitigate certain stack overflow vulnerabilities. Explain further. * Impact on compatibility: NO (or YES if there are compatibility concerns). * **Testing:** This section is significantly lacking. "ci-check" is not sufficient testing information. While requesting help from another developer is acceptable, the PR submitter is still responsible for providing *some* level of testing verification. Even without a physical board, QEMU simulation could be used. The PR should at minimum include: * Build Host(s) Information: Provide details about the build environment used. * Target(s) Information: Specify the esp32s3 configuration used (even if simulated). * Testing logs before change: This is impossible without local testing. Explain why these are missing if true. * Testing logs after change: Likewise, this is necessary. If actual logs are unavailable, describe the *expected* changes in behavior as a result of this change. For example, "After this change, the system should successfully boot without stack overflow errors previously observed in certain configurations." In short, the PR needs more detail and actual testing (or a strong justification for the lack thereof) before it meets the NuttX requirements. -- 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]
