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

   [**\[Experimental Bot, please feedback 
here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues)
   
   
   
   No.  The provided PR description lacks crucial details required by the NuttX 
guidelines.  While it briefly explains the "why" of the change, it's missing 
the "how".
   
   Here's a breakdown of missing information:
   
   * **Insufficient Summary:**  While the summary states the goal, it doesn't 
explain *how* the change achieves this.  Does it involve ignoring the return 
code?  Adding a check?  Modifying existing logic?  The exact mechanism needs to 
be described.  What functional part of the code is changed? (e.g.,  block 
driver, MTD driver, specific file)
   * **Missing Impact Details:**  While marking "RELEASE" suggests a potential 
impact, it's not descriptive enough.  The PR needs to explicitly state YES/NO 
for each impact category and provide descriptions where applicable. For 
example, even if the user impact is "NO", it's good to explicitly state that.  
Even if the *intended* impact is minimal, unexpected side effects are possible. 
 A careful analysis is necessary.  Given that this relates to BIOC_FLUSH, the 
impact on data integrity in certain scenarios needs careful consideration.
   * **Insufficient Testing:** Saying "CI" isn't sufficient.  While CI testing 
is important,  the PR description *must* include specific details about the 
local testing performed.  This includes:
       * **Build Host Details:**  OS, CPU architecture, compiler version.
       * **Target Details:**  Architecture (e.g., simulator, real hardware), 
board, and configuration.
       * **Test Logs:**  "Before" and "after" logs demonstrating the issue and 
its resolution are mandatory.  Simply stating "CI" doesn't provide evidence of 
the fix.
   
   
   In short, the PR description needs to be substantially more detailed to meet 
the NuttX requirements.  It needs to clearly explain the technical 
implementation, analyze the potential impact across all categories, and provide 
concrete evidence of testing.
   


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