nuttxpr commented on PR #15722: URL: https://github.com/apache/nuttx/pull/15722#issuecomment-2623482377
[**\[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, assuming the provided information is accurate and complete. However, some sections need more detail to be truly compliant. Here's a breakdown of what's good and what needs improvement: **Good:** * **Clear Summary:** The summary explains the purpose of the change (a new tool to list watchdog timers) and provides an example of its output. * **Minimal Impact:** The stated impact is minimal, which is often the case for debugging tools. * **Testing Logs Included:** While incomplete, the inclusion of example output demonstrates the tool's functionality. **Needs Improvement:** * **Testing Section:** This is the most significant area for improvement. The placeholder text needs to be replaced with specific details: * **Build Host:** Specify the OS (e.g., Linux Ubuntu 20.04), CPU architecture (e.g., x86_64), and compiler version (e.g., GCC 9.4.0). * **Target:** Specify the architecture (e.g., sim:nsh, arm:stm32f4discovery), board configuration (e.g., defconfig), and any relevant details about the target environment. * **"Before" Logs:** While the example output shows the tool's output *after* the change, there's no "before" equivalent. Ideally, you should show what information was *not* available before this tool was added (e.g., "no command existed to list watchdog timers"). If there was a less convenient way to get this information, show that method. * **Complete Test Procedure:** Describe the steps taken to test the change. Did you manually create some watchdog timers? Did you test under different conditions (e.g., different numbers of timers, timers expiring)? **Example of an Improved Testing Section:** ``` Testing Build Host: Linux Ubuntu 22.04, x86_64, GCC 11.3.0 Target: sim:nsh (qemu-x86_64) Testing logs before change: No command existed to list active watchdog timers. Information could be partially obtained by manually inspecting the wdog structure in GDB, but this was tedious and error-prone. Testing logs after change: (Example provided in Summary section) Test Procedure: 1. Built NuttX for sim:nsh with the new wdog command included. 2. Started the NSH simulator. 3. Created three watchdog timers using wd_start() with different timeout values and callback functions. 4. Ran the 'wdog' command to verify that all three timers were listed correctly, with the expected tick values, callback functions, and arguments. 5. Allowed one of the timers to expire and verified that it was no longer listed by the 'wdog' command. ``` By providing these specific details, you make it much easier for reviewers to understand how you tested your change and to assess its correctness. This leads to faster reviews and a higher likelihood of your PR being accepted. -- 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
