nuttxpr commented on PR #15970: URL: https://github.com/apache/nuttx/pull/15970#issuecomment-2713744801
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) Yes, this PR mostly meets the NuttX requirements, but needs improvement in the "Testing" and "Impact" sections. It's missing specific details required for proper review. Here's a breakdown of what's missing and how to improve it: **Impact:** * **Impact on user:** While you state it impacts only the bootloader, be more specific. Will bootloader users (developers working on the bootloader) need to change anything in their code to use this new ELE handling? If not, explicitly state "NO". * **Impact on build:** Does adding `imx9_ele.c` require any changes to Kconfig, Makefiles, or other build system components? If not, explicitly state "NO". If yes, explain what changes are needed. * **Impact on hardware:** Specify which i.MX93 boards were tested. Is this change specific to *your* custom hardware, or is it generally applicable to all i.MX93 boards? Clarify this. * **Impact on documentation:** Does this change require documentation updates? If so, were these updates made as part of the PR? If no documentation updates are needed, explicitly state "NO". * **Impact on security, compatibility, and other considerations:** Explicitly state "NO" for each of these if they don't apply. **Testing:** * **Build Host(s):** Provide details about your build host OS, CPU architecture, and compiler version (e.g., "Linux, x86_64, GCC 12.2.0"). * **Target(s):** Be specific about the i.MX93 board and configuration used for testing (e.g., "i.MX93-EVK, default config"). Just stating "custom imx93 hardware" is insufficient. * **Testing Logs:** The placeholder logs need to be replaced with actual logs demonstrating the functionality before and after the change. Show how the bug was present before and fixed after. Highlight the new messages added to the ELE API. Demonstrate the updated firmware version. The logs should be concise but provide enough information to verify the changes. If the logs are very long, consider attaching them as a file to the PR instead of embedding them directly. **Example of improved Testing section:** ``` Testing I confirm that changes are verified on local setup and works as intended: * Build Host(s): Linux, x86_64, GCC 12.2.0 * Target(s): Custom i.MX93 board based on i.MX93-EVK, default config Testing logs before change: ``` <Show logs demonstrating the channel bug in imx9_ele_sendmsg() and imx9_ele_receivemsg() and the absence of new messages. Also, show the old firmware version.> ``` Testing logs after change: ``` <Show logs demonstrating the fixed channel bug, the presence of the new messages, and the updated firmware version (1.3.0).> ``` ``` By providing these missing details, you'll ensure your PR is thoroughly reviewed and merged efficiently. -- 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
