nuttxpr commented on PR #3043: URL: https://github.com/apache/nuttx-apps/pull/3043#issuecomment-2761180123
[**\[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, but it's missing some crucial details. While it addresses the sections, the content is too brief for a proper review. **Here's what needs improvement:** * **Summary:** "Fix various coding standard issues" is vague. List the specific coding standard violations that were fixed. What was the actual compilation issue? This helps reviewers understand the scope and nature of the changes. * **Impact:** While functional changes may not exist, you should still specify NO for each impact category explicitly. For example: * Impact on user: NO * Impact on build: NO (except for fixing the compilation issue, which should be elaborated on) * ...etc... * **Testing:** Simply stating "compile with make" is insufficient. Provide: * **Specific build host details:** OS version, compiler version (e.g., "Ubuntu 22.04, GCC 11.3.0"). * **Specific target details:** Architecture, board, configuration (e.g., "sim:qemu-arm"). * **Relevant logs:** "Testing logs before/after" should show that the compilation issue is resolved and demonstrate the intended behavior (even if it's unchanged functionality). Include compiler output (errors before, successful compilation after) and, ideally, runtime output demonstrating the examples still work. **Example of Improved Content:** **Summary:** 1. `examples/nimble_bleprph`: Fixed coding standard violations related to indentation (using tabs instead of spaces), variable naming (using underscores instead of camelCase), and missing comments. The compilation issue was caused by an incorrect include path for a header file (`xyz.h`). 2. `examples/nimble_blecent`: Similar coding standard violations as above were fixed (indentation, naming). The compilation issue stemmed from a missing definition of a macro (`XYZ_DEFINE`). **Impact:** * Impact on user: NO * Impact on build: YES (Previously, `nimble_bleprph` and `nimble_blecent` failed to compile due to the mentioned include and macro issues. This PR fixes the build process, allowing successful compilation.) * Impact on hardware: NO * Impact on documentation: NO * Impact on security: NO * Impact on compatibility: NO **Testing:** * Build Host: Ubuntu 22.04, GCC 11.3.0 * Target: sim:qemu-arm Testing logs before change (nimble_bleprph): ``` examples/nimble_bleprph/src/main.c:12:10: fatal error: xyz.h: No such file or directory #include <xyz.h> ^~~~~~~~ compilation terminated. make: *** [Makefile:xyz_target] Error 1 ``` Testing logs after change (nimble_bleprph): ``` ... (Successful compilation output) ... ``` (Similar logs for nimble_blecent before and after). Ideally, include some output demonstrating the example running correctly after the fix. By providing these details, you'll significantly increase the chances of your PR being reviewed and merged quickly. -- 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
