nuttxpr commented on PR #14039: URL: https://github.com/apache/nuttx/pull/14039#issuecomment-2404595788
[**\[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.** **Missing Information:** * **Summary:** * Lacks a clear explanation of *why* the change is necessary. While the description mentions issues with `libuv` and potential integer overflows, it doesn't clearly articulate the problem being solved and its impact. * Needs to be more specific about the functional part of the code being changed (e.g., which files, functions). * **Impact:** * The impact section is incomplete. It only mentions "FS poll" but doesn't address all the required points: * Is this a new feature, a bug fix, or an optimization? * What is the impact on users? Will they need to adapt to this change? * What is the impact on documentation? Is an update required? * Are there any security implications? * Are there any backward or forward compatibility concerns? * **Testing:** * **Insufficient Detail:** Simply stating "sim & qemu" is not enough. * Specify the host operating system, CPU architecture, and compiler used. * Provide specific details about the simulator configurations. * For QEMU, specify the target architecture and board configuration. * **Missing Logs:** The "Testing logs before change" and "Testing logs after change" sections are empty. Provide actual log output demonstrating the issue before the change and the improvement after the change. **Recommendations for Improvement:** 1. **Clarify the Problem:** In the Summary, clearly state the problem being solved. Explain why the existing `poll` implementation is incorrect according to the `man` page and how it negatively impacts `libuv`. 2. **Explain the Solution:** Provide more detail on *how* the code changes address the problem. What specific modifications are made to `fs_poll.c`? How does `result_independent_of_operands` relate to the overall fix? 3. **Complete the Impact Section:** Address all the points listed in the requirements. Be specific about any potential compatibility issues or required changes for users. 4. **Provide Detailed Testing Information:** Include: * Host OS, CPU, and compiler details. * Simulator/QEMU configurations. * **Actual log output** before and after the change to demonstrate the fix. By providing this missing information, your PR will be much stronger and easier for the NuttX maintainers to review. -- 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]
