nuttxpr commented on PR #2931: URL: https://github.com/apache/nuttx-apps/pull/2931#issuecomment-2574698835
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) No, this PR description does not adequately meet the NuttX requirements. Here's why and how to fix it: **Missing Information in Summary:** * **Why is this change necessary?** Just saying "merge test cases" isn't enough. Explain the benefits (e.g., better organization, easier to find tests, avoids naming conflicts, improves maintainability). * **What functional part of the code is being changed?** Be specific. "apps/testing" is a start, but mention the affected subdirectories or modules. * **How does the change exactly work?** Don't just say they'll be "separated and placed in different folders." Describe the new folder structure. What criteria are used to separate the tests? Provide examples. * **Related Issues:** If this addresses a specific issue in NuttX or NuttX Apps, link to it. **Missing Information in Impact:** * All the "Impact" questions need more specific answers than "NO" or a vague "YES." Even if the answer is "NO," briefly explain why (e.g., "Impact on build: NO, this only affects the organization of source files"). * **Impact on user:** How will this reorganization affect users who run these tests? Will they need to change any commands or scripts? * **Impact on documentation:** If the organization of tests changes, the documentation probably needs to be updated. State that explicitly. **Insufficient Testing Information:** * **Build Host(s):** Specify your development environment (OS, CPU architecture, compiler version). * **Target(s):** List the specific architectures and boards you tested on (e.g., `sim:nsh`, `arm:stm32f4discovery:hello`). * **Testing logs:** Provide *actual* logs, not just "Local self-test can be done through." Show the test output before and after the change to demonstrate that the tests still pass and that the reorganization hasn't broken anything. If the change *is* about fixing tests, show how the output has improved. **Example of a Better PR Description:** ## Summary This PR reorganizes the test cases under `apps/testing` to improve maintainability and findability. Currently, all test cases are in a single directory, making it difficult to navigate and potentially leading to naming conflicts. This change groups related tests into subdirectories based on their functionality (e.g., networking tests, file system tests, driver tests). This addresses [NuttX Apps Issue #XYZ] (if applicable). ## Impact * **Is new feature added?** NO * **Is existing feature changed?** YES, the organization of test cases is changed. * **Impact on user:** YES. Users running these tests will need to update their commands or scripts to reflect the new directory structure. The README in `apps/testing` will be updated with the new organization. * **Impact on build:** NO, this only affects the organization of source files, not the build process itself. * **Impact on hardware:** NO, this change does not affect any hardware or drivers. * **Impact on documentation:** YES, the README in `apps/testing` will be updated to describe the new directory structure. * **Impact on security:** NO * **Impact on compatibility:** NO, existing tests should continue to function as before. * **Anything else to consider:** N/A ## Testing I confirm that changes are verified on my local setup and work as intended: * **Build Host:** macOS Ventura, Intel x86_64, GCC 12.2 * **Target(s):** `sim:nsh`, `arm:stm32f4discovery:hello` **Testing logs before change:** ``` ls apps/testing test_case1.c test_case2.c test_case3.c ... ./apps/testing/test_case1 ... test output ... ``` **Testing logs after change:** ``` ls apps/testing networking/ filesystem/ drivers/ ls apps/testing/networking test_case1.c test_case2.c ./apps/testing/networking/test_case1 ... test output ... ``` By providing more details and specific information, you make it much easier for reviewers to understand and approve your PR. Remember to replace the placeholders with your actual information. -- 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]
