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

   [**\[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 itself, it lacks crucial information.
   
   Here's a breakdown of what's missing:
   
   * **Summary:**  While the title describes *what* was changed, it doesn't 
explain *why*. What problem did the unpaired spinlock cause?  What part of the 
`esp/mcpwm` code was affected? How does the fix work?  A related issue number 
should be included if one exists.
   
   * **Impact:**  Simply stating "RELEASE" is insufficient.  Each impact 
category (user, build, hardware, documentation, security, compatibility) needs 
to be addressed with either "NO" or a "YES" followed by a description.  For 
example, even if the impact on the user is minimal,  the description might say 
"Users relying on the `esp/mcpwm` driver may see improved stability."  The 
security impact, especially for a lock-related fix, should be carefully 
considered.
   
   * **Testing:** "CI" is not enough.  The PR needs to demonstrate that the 
author tested the changes *before* submitting.  This requires specifying the 
build host and target environment, and providing "before" and "after" logs that 
demonstrate the issue and its resolution.  Relying solely on CI testing is not 
sufficient for demonstrating that the author has validated their changes.
   
   
   In short, the PR needs more detail and evidence of testing to meet the NuttX 
requirements.
   


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