nuttxpr commented on PR #2845:
URL: https://github.com/apache/nuttx-apps/pull/2845#issuecomment-2472669143

   [**\[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 and mentions testing, it lacks crucial information.  Here's a breakdown:
   
   * **Insufficient Summary:**  "stdio.h:remove <nuttx/xxx.h>" is vague.  
*Which* `nuttx/xxx.h` file was removed? *Why* was it removed?  What problem did 
it solve? What functionality does this change?  This needs significant 
expansion.
   
   * **Impact Section Severely Lacking:** The statement "all use stdio.h" 
doesn't explain the impact.  It needs to address *all* the points in the 
template, even if the answer is "NO."  Explicitly stating "NO" with brief 
justification is better than silence.  For example:
       * **Impact on user:** NO (This is an internal header change and should 
not affect user applications directly).
       * **Impact on build:**  Potentially YES (If any code relied on the 
removed include, it might now fail to compile. This needs investigation and 
explanation).
       * **Impact on hardware:** NO (This change does not affect hardware 
directly).
       * ...and so on for all impact categories.
   
   * **Testing Insufficient:** "sim:citest build and sim:nsh build" is too 
brief. Provide actual log output (even if it's just a successful build 
message).  What tests were run within `citest` and `nsh` to confirm the change 
works correctly and doesn't break anything? More details are needed about the 
build host as well.
   
   * **Missing "Before" Logs:**  The template requires "Testing logs before 
change," which are absent.
   
   In short, the PR needs to be much more thorough in explaining the context, 
rationale, impact, and testing results.  Simply stating what changed isn't 
enough; it needs to justify *why* the change is necessary and demonstrate that 
it has been thoroughly vetted.
   


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

Reply via email to