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

   [**\[Experimental Bot, please feedback 
here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues)
   
   
   
   No, this PR description does not fully meet the NuttX requirements.  While 
it addresses the summary and testing sections somewhat, it lacks crucial detail 
and completely omits the "Impact" section details.
   
   Here's what's missing and needs to be added:
   
   **Summary:**
   
   * **More specific "what" and "how":**  Instead of "refactored the directory 
adding logic," describe *what* logic was refactored and *how* it was changed.  
Be explicit.  For example: "The `add_directory_to_romfs` function previously 
incorrectly calculated offsets when nested directories were present. This patch 
corrects the offset calculation by..."
   * **More specific "why":**  *Why* was the original directory adding logic 
broken? Was it a recent regression? A long-standing bug? This helps reviewers 
understand the context.
   * **Link related issues:** If this fixes a reported bug, link the NuttX 
issue.
   
   
   **Impact:**  This section is essentially empty.  You need to fill out *all* 
the sub-points.  Even if the answer is "NO", explicitly state it. For example:
   
   * **Is new feature added? Is existing feature changed?**: No.  Bug fix.
   * **Impact on user**: YES.  Previously, users could not add raw directories 
to ROMFS. This is now fixed.
   * **Impact on build**: NO.
   * **Impact on hardware**: NO.
   * **Impact on documentation**:  Maybe YES if the bug fix warrants a 
documentation update or clarification. Otherwise, NO.
   * **Impact on security**: NO. (Unless the bug had security implications).
   * **Impact on compatibility**: NO (Unless this fix changes behavior that 
existing users rely on).
   * **Anything else to consider?**: NO.
   
   
   **Testing:**
   
   * **More detail on build host:** Be more specific. "Linux, Ubuntu 22.04, GCC 
11.2.0" is much better than just "Linux."
   * **More detail on target:** "sim:nsh" is okay, but adding more context like 
the simulator being used (e.g., QEMU) would be helpful.
   * **Show *relevant* logs:** "build pass" isn't helpful.  Show logs 
demonstrating the *problem* before the patch and the *corrected behavior* after 
the patch.  Focus on the specific functionality being fixed. For example, show 
attempts to mount and access the ROMFS directory before and after the fix.
   
   
   
   By providing this level of detail, you make it significantly easier for 
reviewers to understand, evaluate, and ultimately merge your PR. Remember, 
clarity and completeness are key to a successful contribution.
   


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