nuttxpr commented on PR #2881: URL: https://github.com/apache/nuttx-apps/pull/2881#issuecomment-2523848994
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) __Fill In The Commit Message:__ This PR contains a Commit with an Empty Commit Message. Please fill in the Commit Message with the PR Summary. No, this PR summary does not fully meet the NuttX requirements. Here's why and how to fix it: * **Insufficient Detail in Summary:** While it mentions new features, it lacks crucial details. It needs to explain *why* these changes are necessary. What problem are they solving? How exactly do the new features "fetch diagnostic data in different ways"? What is the benefit of not using esptool? Be specific. * **Missing Issue References:** Are there related NuttX or NuttX Apps issues that prompted this PR? If so, provide links. * **Impact Section Too Brief:** The impact section is far too general. You must address *each* bulleted point individually. For example: * **Is new feature added?**: YES * **Is existing feature changed?**: Potentially YES (moving verbose option). Describe what changed about the existing feature. * **Impact on user**: YES (users can now get diagnostic data without esptool). Explain how this impacts the user workflow. Are there new commands? New configuration options? * **Impact on build**: Possibly YES. Explain *how* the build process changes due to "dignostic info logging support during build." Are there new build options? New dependencies? * **Impact on hardware**: Specify which architectures/boards are affected (Espressif). Are there any hardware dependencies introduced? * **Impact on documentation**: YES (almost certainly, since new features are added). Describe what documentation needs to be updated or what new documentation is provided. * **Impact on security**: Even if the answer is NO, explicitly state it. If YES, provide a detailed explanation. * **Impact on compatibility**: Explicitly address backward/forward/interoperability concerns. * **Insufficient Testing Information:** "esp32c6-devkitc:nsh with `NXDIAG` tests options enabled" is not enough. You must specify: * **Build Host:** Your OS, CPU architecture, compiler version. * **Target(s) in Detail:** Be specific. "esp32c6-devkitc:nsh" is a start, but include the NuttX configuration used (e.g., `nsh`). * **Testing Logs:** The template specifically requests "Testing logs before change" and "Testing logs after change." You must provide these logs to demonstrate the functionality and the improvement provided by your changes. Don't just say tests passed; show the output. **Example of an Improved Impact Section:** * **Is new feature added?**: YES, added diagnostic info logging during the build process and the ability to fetch diagnostic data without esptool. * **Is existing feature changed?**: YES, the verbose option has been moved to be commonly used across different diagnostic methods. Previously it was specific to [explain previous location/usage]. * **Impact on user**: YES, users can now trigger diagnostic logging during the build process using [explain how, e.g., a new Kconfig option] and retrieve diagnostic data without esptool using the new command `nxdiag fetch` [explain usage and options]. * **Impact on build**: YES, enabling diagnostic logging during the build will increase build time slightly and add a new dependency on [mention the dependency, if any]. A new Kconfig option `CONFIG_NXDIAG_BUILD_LOGGING` controls this feature. * **Impact on hardware**: YES, these changes specifically affect Espressif devices. No new hardware dependencies are introduced. * **Impact on documentation**: YES, the documentation will be updated to describe the new build logging feature, the `nxdiag fetch` command, and the relocated verbose option. New documentation will be added to the `apps/nxdiag` README. * **Impact on security**: NO. * **Impact on compatibility**: Backward compatible with existing nxdiag functionality. No forward compatibility concerns. No known interoperability issues. By providing more specific information in each section, you'll make it much easier for reviewers to understand and evaluate your PR. This will significantly increase the chances of it 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
