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

   [**\[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 and testing information, it lacks crucial details.
   
   Here's what's missing:
   
   * **Summary:**  The summary is too brief.  It needs to explain *why* moving 
`up_set_interrupt_context()` to a chip-specific definition is necessary. What 
problem does this solve? What are the benefits?  It mentions "correct place," 
but doesn't explain what makes that location correct.
   * **Impact:**  Simply stating "N/A" is insufficient.  The author needs to 
thoughtfully consider and address each impact point. Even if there's no impact, 
they should explicitly state "NO" for each item.  For example, there might be 
an impact on hardware (specific architectures) or documentation (if this 
function was previously documented elsewhere).  Dismissing the entire Impact 
section suggests a lack of thoroughness.
   * **Testing:** "ci-check" is not sufficient testing information.  While CI 
is important, the PR needs to provide specific details about the local testing 
performed.  This includes:
       * The specific build host operating system, CPU architecture, and 
compiler version used.
       * The specific target architecture, board, and configuration tested.
       * Actual testing logs (or a link to them if they are extensive) 
demonstrating the behavior before and after the change.  "ci-check" doesn't 
provide any evidence of the change's effectiveness.
   
   
   In short, the PR needs to be significantly more detailed and address all the 
required points in the template to be considered complete.
   


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