nuttxpr commented on PR #2737: URL: https://github.com/apache/nuttx-apps/pull/2737#issuecomment-2413876099
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) The PR partially meets the NuttX requirements. **Strengths:** * **Summary** provides a clear explanation of the changes and their purpose. * **Testing** section includes specific examples and mentions CI, which is good. **Areas for improvement:** * **Impact:** * The impact section is incomplete. It only mentions `nsh: nsh_parse_command()` without explaining the impact of the change on users, builds, hardware, documentation, security, or compatibility. * You need to thoroughly address all impact points, stating "NO" or "YES" with a clear and concise description if applicable. For instance: * **Impact on the user:** Will users need to change how they use the `nsh` command line due to pipeline support? * **Impact on build:** Will any build system changes be needed for this feature? * **Impact on documentation:** Does this change require documentation updates? If yes, have you updated the documentation? * **Testing:** * While you mention "Selftest" and "Apache:NuttX CI," providing snippets of the actual logs (before and after the change) would significantly strengthen this section. * Detailing the build host and target environments used for testing would also be beneficial. For example: * **Build Host:** Ubuntu 20.04, GCC 9.4 * **Target:** SIM, ARM Cortex-M4 **Recommendations:** 1. **Expand the Impact section.** Provide a "NO" or "YES" answer and a brief explanation for each impact point. 2. **Include detailed testing logs.** Add snippets of the actual testing logs before and after your changes for both the selftest and CI environments. 3. **Specify testing environments.** Clearly state the build host operating system, compiler, and target architecture/board used for testing. 4. **Consider future work:** Briefly mention your plans to upstream nested pipeline support. This demonstrates ongoing commitment and a roadmap for future improvements. By addressing these points, your PR will be more comprehensive and easier for the NuttX maintainers to review. -- 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
