nuttxpr commented on PR #2942: URL: https://github.com/apache/nuttx-apps/pull/2942#issuecomment-2588681253
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) No. This PR description, while providing some information, is insufficient to meet the NuttX requirements. Here's why and how it can be improved: * **Insufficient Summary:** "Add test for net bufpool" is too vague. Which part of the networking code is being tested? What specific functionality of the bufpool is being tested? *How* does the test work? What kind of test is it (unit, integration, system)? A better summary would be something like: "Adds a unit test to verify correct allocation and freeing behavior of network buffers in the bufpool when under high load. This test specifically targets the `xxx` function and checks for double-frees and memory leaks." * **Impact: "N/A" is almost never the correct answer.** While the impact *might* be minimal, you still need to consider and explicitly state the impact on things like documentation (does the test need to be documented?), build system (does the test need to be added to the build system?), and potentially even compatibility (does this test expose a bug that affects compatibility?). Even if the answer is "NO" for all of these, you need to explicitly state it. Don't just say "N/A". * **Insufficient Testing Details:** "local qemu test" is not enough information. Which QEMU architecture? Which NuttX configuration? What specific commands were used for testing? What were the expected results? The "Testing logs before change" and "Testing logs after change" sections are empty. You MUST provide concrete evidence that the changes work as intended. Include relevant log output, ideally showing the test failing *before* your change and passing *after* your change. Specify the host operating system, compiler version, and any other relevant details about your testing environment. For example: ``` Build Host: Linux (Ubuntu 22.04), x86_64, GCC 11.2.0 Target: QEMU-RV32M, `nsh` configuration Testing logs before change: ``` nsh> net_test ... test output showing failure ... ``` Testing logs after change: ``` nsh> net_test ... test output showing success ... ``` In short, be explicit and thorough in each section of the PR template. Provide enough detail for a reviewer to understand the changes, their impact, and how they were tested. Don't assume anything is obvious. -- 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]
