nuttxpr commented on PR #15183:
URL: https://github.com/apache/nuttx/pull/15183#issuecomment-2541628837

   [**\[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 description does not fully meet the NuttX requirements.  While 
it provides a decent summary, it lacks crucial information in the Impact and 
Testing sections.
   
   Here's a breakdown of what's missing and how to improve it:
   
   **Missing Information:**
   
   * **Impact:** While the affected architectures are mentioned, the other 
impact categories are completely empty.  The PR author *must* address *all* 
impact questions, even if the answer is "NO".  For example, if there's no user 
impact, explicitly state "Impact on user: NO". If the build process doesn't 
change, state "Impact on build: NO". This confirms the author considered these 
aspects.  For the architectures mentioned, *describe* the impact. Is it a bug 
fix? A new feature?  Does it improve performance?
   * **Testing:** This section is entirely empty. The author *must* provide:
       * Details of their build host environment (OS, CPU, Compiler).
       * Details of the target hardware or simulator used.
       * Most importantly, *actual testing logs* from before and after the 
change.  These logs should demonstrate the error occurring before the fix and 
its absence after the fix. Without these, the PR cannot be properly reviewed.
   
   
   **How to Improve:**
   
   **Impact Example:**
   
   ```
   * Is new feature added? NO
   * Is existing feature changed? YES. Fixes a build error when 
CONFIG_IMXRT_LPUART_SINGLEWIRE and CONFIG_IMXRT_LPUART_INVERT are enabled 
without CONFIG_SERIAL_TERMIOS or CONFIG_SERIAL_TIOCSERGSTRUCT.
   * Impact on user:  NO.  Users who were encountering the build error will now 
be able to build successfully.
   * Impact on build: NO. The build process itself is unchanged, only the build 
outcome for certain configurations is affected.
   * Impact on hardware: YES. This fix affects arm/imxrt, arm/s32k3xx, and 
arm64/imx9 architectures by correcting the build issue.  This should enable the 
proper functionality of the SINGLEWIRE and INVERT features on these platforms.
   * Impact on documentation: NO.
   * Impact on security: NO.
   * Impact on compatibility: YES.  This fix restores backward compatibility by 
allowing the use of the affected configurations.
   * Anything else to consider? NO.
   ```
   
   
   **Testing Example:**
   
   ```
   I confirm that changes are verified on local setup and works as intended:
   * Build Host(s): Linux (Ubuntu 22.04), x86_64, GCC 11.2.0
   * Target(s): sim:imxrt1060-evk,  arm/imxrt:imxrt1060-evk (physical hardware)
   
   
   Testing logs before change:
   
   ```
   /path/to/nuttx/configs/imxrt1060-evk/nsh/Make.defs:108: recipe for target 
'drivers/serial/imxrt_lpuart.o' failed
   make[2]: *** [drivers/serial/imxrt_lpuart.o] Error 1
   ...
   'cfsetospeed' undeclared here (not in a function); did you mean 
'cfsetispeed'?
   ```
   
   Testing logs after change:
   
   ```
   ...
   make[1]: Leaving directory '/path/to/nuttx/nuttx'
   Successfully built NuttX
   ```
   ```
   
   
   By providing this additional information, the PR will meet the NuttX 
requirements and be much easier for reviewers to assess and merge.
   


-- 
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

Reply via email to