nuttxpr commented on PR #15187: URL: https://github.com/apache/nuttx/pull/15187#issuecomment-2541951194
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) No, this PR does not fully meet the NuttX requirements. While it provides a summary of the change, it lacks crucial information. Here's a breakdown of what's missing: * **Summary:** While it states *what* is being done (providing nimble config), it doesn't explain *why* (is it a new feature, bug fix, etc.), *how it works* (details of the nimble configuration), or reference related issues. * **Impact:** This section is completely empty. It needs to address all the listed points (impact on user, build, hardware, documentation, security, compatibility). Even if the answer is "NO" for some, it should be explicitly stated. * **Testing:** While the host OS is mentioned, compiler information is missing. Crucially, the target information is incomplete. While `esp32c3` is mentioned, the specific board and configuration are not. And most importantly, there are no "before" and "after" testing logs. These logs are essential to demonstrate the change's effect and that it works as intended. To make this PR compliant, you need to add the missing information in each section. For example: **Example of a more complete Summary:** > This change adds nimble configuration for the esp32c3 to enable Bluetooth Low Energy (BLE) functionality. It adds a new Kconfig file for nimble under the esp32c3 architecture and updates the board configuration to enable it. This addresses [NuttX Issue #XXX](link-to-issue) which requested BLE support for esp32c3. **Example of a more complete Impact:** > * Is new feature added? YES (Adds BLE support via nimble) > * Impact on user (will user need to adapt to change)? YES (Users will need to enable nimble in their configuration and may need to install additional tools for BLE development) > * Impact on build (will build process change)? YES (Nimble will be included in the build if enabled) > * Impact on hardware (will arch(s) / board(s) / driver(s) change)? YES (Affects esp32c3 architecture and boards using it) > * Impact on documentation (is update required / provided)? YES (Documentation will need to be updated to describe how to configure and use nimble on esp32c3) > * Impact on security (any sort of implications)? NO > * Impact on compatibility (backward/forward/interoperability)? NO > * Anything else to consider? None **Example of more complete Testing:** > I confirm that changes are verified on local setup and works as intended: > * Build Host(s): OS (Ubuntu 22.04 on WSL2), CPU(x86_64), compiler(GCC 11.3.0) > * Target(s): arch(esp32c3), board:esp32c3-devkit:default > Testing logs before change: > ``` > (Log output showing the lack of BLE functionality, e.g., attempting a BLE scan and failing) > ``` > Testing logs after change: > ``` > (Log output showing successful BLE operation, e.g., successful scan results) > ``` By filling in the missing information like this, your PR will better meet the requirements and be easier for reviewers to assess. -- 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
