nuttxpr commented on PR #2974: URL: https://github.com/apache/nuttx-apps/pull/2974#issuecomment-2611710788
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) No, this PR description does not fully meet the NuttX requirements. While it provides a summary and mentions testing, it lacks crucial details. Here's a breakdown of what's missing and how to improve it: **Summary:** * **Missing "Why":** The summary states *what* the change is but not *why*. Is this for better organization? Does it fix a problem or enable something new? Explain the rationale behind grouping these specific tests under "sched". * **Missing "How":** The summary mentions moving folders, but not *how* this was done. Were files simply moved? Were any build scripts or configurations modified? **Impact:** While the impact section addresses some points, it lacks detail and omits others: * **Impact on user:** While the user is likely a developer, the impact should still be described. Will any existing test scripts need to be updated to reflect the new paths? * **Impact on build:** The impact on the build process should be clarified. Does simply moving the folders affect anything in the build system (e.g., Makefiles, CMakeLists.txt)? Even if the answer is NO, explicitly state it and briefly explain why (e.g., "No, because the build system uses relative paths"). * **Impact on hardware:** This is likely NO, but should be explicitly stated. * **Impact on documentation:** If the move requires any documentation updates (e.g., tutorials, READMEs), mention them here. If no updates are needed, state that explicitly. * **Impact on security:** This is likely NO, but should be explicitly stated. * **Impact on compatibility:** This is likely NO, but should be explicitly stated. **Testing:** * **Insufficient Detail:** "CI test" is not enough. Specify the CI environment (e.g., GitHub Actions, Travis CI). Which targets were tested? Were *all* tests under these moved folders run successfully? * **Missing Logs (or explanation):** The template requests "Testing logs before change" and "Testing logs after change". Since this is a structural change, full logs might be excessive. However, you should *at least* explain *what* was tested and *how* success was determined. For example: "All tests under `atomic`, `cpuload`, `getprime`, `smp`, and `timerjitter` were run successfully on the `qemu-x86_64` target before and after the folder move, confirming that the relocation did not break the tests." **Example of an improved PR description:** ``` ## Summary This change moves several test folders related to scheduling concepts (atomic operations, CPU load, prime number calculation, SMP, and timer jitter) into a new "sched" subdirectory within the `apps/testing` folder. This improves organization and makes it easier to locate tests related to scheduling functionality, as discussed in https://github.com/apache/nuttx-apps/pull/2931. The folders were moved using standard file system commands, and build scripts were updated to reflect the new paths. ## Impact * **Impact on user:** YES. Developers running these tests will need to update their scripts to reflect the new paths. * **Impact on build:** YES. Makefiles within the `apps/testing` directory were updated to reflect the new folder structure. * **Impact on hardware:** NO. This change is purely organizational within the test suite. * **Impact on documentation:** YES. The README file for the `apps/testing` directory will be updated to reflect the new folder structure. * **Impact on security:** NO. This change has no impact on security. * **Impact on compatibility:** NO. This change maintains backward compatibility. Existing test scripts can be easily updated to accommodate the new paths. ## Testing I confirm that changes are verified on a local setup and work as intended: * Build Host: Linux (Ubuntu 20.04), x86_64, GCC 9.4.0 * Target: qemu-x86_64 Testing Description: All tests under the `atomic`, `cpuload`, `getprime`, `smp`, and `timerjitter` folders were executed successfully on the `qemu-x86_64` target both before and after moving the folders. This confirms that the tests remain functional after the reorganization. Specific test logs are omitted as they are extensive and unchanged functionally. The CI system will also run these tests to ensure broader platform coverage. ``` This improved description provides a much clearer and more complete picture of the changes and their impact, fulfilling the requirements of a NuttX PR. Remember to tailor the specifics to your actual changes and testing. -- 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
