nuttxpr commented on PR #15258: URL: https://github.com/apache/nuttx/pull/15258#issuecomment-2550739477
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) This PR does **not** fully meet the NuttX requirements. While it provides a summary of the changes, it lacks crucial information in the Impact and Testing sections. Here's a breakdown of what's missing and how it can be improved: **Missing Information:** * **Impact:** The Impact section is entirely placeholder text. The author needs to specify how this change impacts users, the build process, hardware, documentation, security, and compatibility. Since this PR involves `fork()`, there's potential for significant impact, and it's crucial to document these changes. For example, does this fix a bug that was causing crashes? Does it introduce any new limitations? Does it change the behavior of `fork()` in a way that existing applications might not expect? * **Testing:** The Testing section is also placeholder text. The author *must* provide details about their testing environment (host OS, compiler, target architecture, board, configuration). Critically, they need to include logs demonstrating the problem *before* the change and how the change fixes the problem. The current snippets of assembly are not helpful without context. Show the full error messages or problematic behavior. **How to Improve the PR:** 1. **Expand the Impact section:** Address *every* item in the Impact section, even if the answer is "NO". If the answer is "YES," provide a detailed explanation. For example: * **Impact on user:** Will users need to change their code to use the updated `fork()`? * **Impact on build:** Are any new configuration options introduced? * **Impact on hardware:** Does this change affect specific architectures or boards? * **Impact on documentation:** Does the documentation need to be updated to reflect this change? * **Impact on security:** Does this fix address a security vulnerability? * **Impact on compatibility:** Does this change break compatibility with any existing code? 2. **Provide detailed testing information:** * Specify the host operating system, compiler version, and any relevant build tools. * Specify the target architecture, board, and NuttX configuration. * Include *complete* logs demonstrating the issue before the change and the corrected behavior after the change. Don't just include snippets of assembly. Show the program output, error messages, or other observable behavior. Ideally, provide a minimal reproducible example that demonstrates the issue. **Example of Improved Testing Section:** ``` Testing Build Host: Linux (Ubuntu 20.04), x86_64, GCC 9.4.0 Target: QEMU-based simulator, ARM Cortex-M4, `configs/sim/arm/sim/defconfig` Testing logs before change: ``` $ ./my_gcov_test_app Segmentation fault (core dumped) ``` Testing logs after change: ``` $ ./my_gcov_test_app Child process forked successfully. Parent process continuing... ``` The `my_gcov_test_app` uses `fork()` and previously crashed with a segmentation fault due to [explain the reason, referencing the assembly if relevant]. After the change, the application runs correctly, demonstrating that the `fork()` issue has been resolved. The application source code is available at [link to application if possible, or include as a separate file in the PR]. ``` By providing this level of detail, the PR will be much easier to review and merge. It will also help ensure that the changes are thoroughly tested and do not introduce new problems. -- 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]
