nuttxpr commented on PR #15083: URL: https://github.com/apache/nuttx/pull/15083#issuecomment-2527187664
[**\[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 and what's missing: * **Summary:** While the summary describes the problem and solution, it lacks specific details about *what* functional part of the code changed (e.g., "the `imx9_sdhc` driver's interrupt handling and timeout logic"). It also needs to explicitly state that it's a bug fix. Linking a related NuttX issue is strongly recommended. * **Impact:** While it mentions fixing a corner case, it needs to be more explicit. Specifically, address *all* the impact points: * **Is new feature added? Is existing feature changed?** Existing feature changed (bug fix). * **Impact on user:** YES (Fixes potential data corruption or hangs when using SD cards, especially slow ones). * **Impact on build:** NO * **Impact on hardware:** YES (Specific to i.MX9 boards with SD card support). * **Impact on documentation:** Likely NO (Unless the bug was documented behavior). If no documentation changes are needed, explicitly state "NO". * **Impact on security:** Potentially YES (Data corruption due to a race condition could have security implications if exploited. This needs further explanation or marked as NO if not applicable). * **Impact on compatibility:** NO (should be backward compatible). * **Anything else to consider:** NO (unless there is). * **Testing:** Insufficient. * **Build Host(s):** Missing OS, CPU, compiler details. * **Target(s):** More details than "one custom i.MX9 board" are needed. Provide the board configuration name. "sim" should be tested if possible. * **Testing logs before change:** CRITICAL MISSING. Provide logs demonstrating the race condition and its effects. * **Testing logs after change:** CRITICAL MISSING. Provide logs showing the problem is resolved. Just saying "sdstress ran over the weekend" doesn't prove the fix is effective. Show specific tests that triggered the issue before and now pass. **In short:** The PR needs more detail and, critically, needs *evidence* in the form of before/after test logs showing the bug and its resolution. Without this, reviewers cannot properly assess the correctness and impact of the change. -- 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
