nuttxpr commented on PR #15899: URL: https://github.com/apache/nuttx/pull/15899#issuecomment-2678304063
[**\[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. While it provides a summary of *what* the change does, it lacks crucial information: * **Missing Explanation of Necessity:** *Why* is this change necessary? Is it fixing a bug? Is it improving performance? What was the problem before the change? This is the most important part of the summary. * **Missing Scope:** What functional part of the code is being changed? Be specific (e.g., "socket option handling", "network stack"). * **Missing Technical Detail:** *How* does the change work? What specific code was modified? What are the mechanics of the fix/improvement? * **Missing Issue References:** Are there any related NuttX issues? Even if there aren't, state that explicitly (e.g., "No related issues"). * **Insufficient Impact Assessment:** "N/A" is rarely acceptable. Think carefully. Even seemingly small changes can have unexpected impacts. Address all impact categories specifically, even if the answer is "NO". Justify your "NO" answers briefly (e.g., "Impact on build: NO - This change only modifies source code within the existing networking module and does not affect the build process"). * **Missing Testing Detail:** "qemu:local" is not enough. What OS and compiler was qemu running on? What NuttX configuration was tested? Be specific about the target architecture and board configuration. * **Absolutely Missing Test Logs:** Providing "before" and "after" logs is essential. Show evidence that the change fixes a problem or implements a new feature. Without logs, there's no way to verify the PR's claims. **Example of an Improved Summary:** "Fixes a potential crash in `si_getsockopt` when invalid socket options are passed. Previously, parameters were not checked at the top level, leading to possible memory corruption or other undefined behavior when attempting to assign invalid values. This change adds parameter validation at the beginning of the function to catch these errors before any assignments occur." **Example of Improved Testing:** "Build Host: Linux (Ubuntu 22.04), x86_64, GCC 11.2.0 Target: qemu-system-arm, sim:nsh Testing logs before change: ``` nsh> some_command_that_triggers_the_bug Segmentation fault ``` Testing logs after change: ``` nsh> some_command_that_triggers_the_bug Invalid socket option ```" By providing this level of detail, reviewers can quickly understand the purpose, scope, and impact of your changes and verify that they work as intended. This makes the review process much smoother and increases the likelihood of your PR being accepted. -- 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