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

   [**\[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 details. Here's a breakdown:
   
   * **Insufficient Summary:**  "Use small lock to protect usbdev and endpoint 
in mips" is too vague.  *Why* was this lock necessary? What problem did it 
solve?  What specific USB device and endpoint are affected?  It should also 
mention related issues if any exist.
   * **Missing Impact Assessment:** The "Impact" section is severely lacking. 
Simply stating the file path is not enough.  It needs to address *all* the 
impact points explicitly (user, build, hardware, documentation, security, 
compatibility).  For each, it should clearly state YES or NO and provide a 
description if YES.  Even if the answer is NO for all, it should explicitly 
state that.
   * **Inadequate Testing:**  "CI" is not sufficient. While CI testing is 
important, the PR needs to show *specific* test results demonstrating the fix.  
It should include relevant logs from *before* the change showing the problem, 
and logs from *after* the change demonstrating the fix. The build host and 
target details are also missing. Just stating "mips" is not enough; the 
specific board and configuration should be mentioned.
   
   **Example of how the testing section should look:**
   
   ```
   Testing
   
   Build Host(s): Linux (Ubuntu 20.04), x86_64, GCC 9.4.0
   Target(s): MIPS sim:pic32mx795f512l
   
   Testing logs before change:
   
   ```
   <logs showing the problem, e.g., data corruption, crashes, etc.>
   ```
   
   Testing logs after change:
   
   ```
   <logs showing the corrected behavior>
   ```
   
   **In short, the PR needs to be significantly more detailed to meet the NuttX 
requirements.**  It should provide a comprehensive explanation of the change, 
its impact, and clear evidence that the change works as intended.
   


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