nuttxpr commented on PR #15691:
URL: https://github.com/apache/nuttx/pull/15691#issuecomment-2613838685
   [**\[Experimental Bot, please feedback 
here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues)
   
   
   
   Yes, this PR appears to meet the basic NuttX requirements, but it is 
incomplete.  While it provides a summary of the change, its impact, and some 
testing information, it lacks crucial details.
   
   **Missing Information/Weaknesses:**
   
   * **Summary:** While the summary explains the *why*, it lacks sufficient 
detail on *what* precisely changed in the code.  "Use raw_spin_[un]lock to 
replace spin_[un]lock" is a high-level description. More detail on the specific 
code modifications is needed.
   * **Impact:**  Specifying "xtensa" is too broad. List the specific xtensa 
boards affected. Explain the original regression more clearly (what 
functionality was broken?).  Address *all* impact points (user, build, 
hardware, documentation, security, compatibility).  Even if the answer is "NO," 
explicitly state it for each.  The current PR only addresses hardware impact.
   * **Testing:** The testing section is inadequate.  "esp32s3-devkit:smp"  is 
insufficient. Provide:
       * **Complete build information:**  Host OS, compiler version, NuttX 
configuration.
       * **Clearer test procedure:** What steps were taken to test?  How was 
the fix validated?
       * **Actual logs:**  The placeholders for "Testing logs before change" 
and "Testing logs after change" need to be filled with real log output 
demonstrating the issue and the fix.  Simply stating that something "works as 
intended" isn't convincing.  Show evidence.
   * **Incomplete Fix Acknowledgment:** The statement "This patch hasn't 
completely resolved the boot-up issue" is a major red flag. A PR should 
generally fix the issue it claims to address. If it doesn't fully resolve the 
problem, explain why and what further work is needed.  This might warrant 
splitting the fix into multiple, smaller PRs.
   
   
   **Recommendation:**
   
   The PR needs substantial revision before it can be considered for merging.  
Provide the missing information outlined above to make it complete and 
convincing.  If the fix is indeed incomplete, clearly explain the next steps 
and consider breaking down the work into smaller, manageable PRs.
   


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