nuttxpr commented on PR #15889: URL: https://github.com/apache/nuttx/pull/15889#issuecomment-2676141653
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) Yes, this PR appears to meet the basic NuttX requirements, although the provided information is minimal. Here's a breakdown of why and what's missing: **Strengths:** * **Clear Summary of the "Why":** It explains the bug (modifying input actions and incorrect `flink` in `posix_spawn`) and the resulting crash. * **Impact is stated:** The impact on the user (crash during `adb shell`) is described. * **Testing is mentioned:** Specific targets and tests are listed. **Weaknesses & Missing Information:** * **Insufficient Detail in Summary:** The "What" and "How" are not clearly explained. *What* part of the code is changed to fix the bug? *How* does the fix prevent modification of the input actions and correct the `flink` issue? * **Missing Issue References:** Are there related NuttX or NuttX Apps issues? These should be linked. * **Incomplete Impact Assessment:** Many impact fields are missing. Consider: * **Impact on build:** Did the build process change at all due to this fix? * **Impact on hardware:** While the impact on *users* of a specific hardware config (goldfish v7a) is mentioned, is there a wider hardware impact? * **Impact on documentation:** Does this fix require documentation updates? * **Impact on security, compatibility:** These are crucial and must be considered. Even if the answer is "NO," explicitly state it. * **Insufficient Testing Details:** "ci-test" is vague. Which CI tests? Providing snippets of relevant log output *before* and *after* the change is essential to demonstrate the fix. Simply stating "Testing logs before/after change" without the actual logs is unhelpful. What specific tests within `ostest` were run? What commands were issued via `adb shell` to trigger the crash before the fix? What commands were issued after the fix to demonstrate the corrected behavior? **Recommendation:** The PR needs significant improvement in providing details, especially in the Summary, Impact, and Testing sections. Be explicit and provide concrete information. The more detail you give, the easier it will be for reviewers to understand and approve your changes. -- 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
