lupyuen commented on PR #13518:
URL: https://github.com/apache/nuttx/pull/13518#issuecomment-2357572177

   [**\[Experimental Bot, please feedback 
here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13494&type=pullrequests)
   
   Let's break down whether this Pull Request (PR) description meets the NuttX 
requirements.
   
   **Strengths:**
   
   * **Summary:** You clearly state the problem ("sabre-6quad boot failed with 
kernel mode") and imply it's a fix. However, it would be even better to briefly 
explain *why* it failed.
   * **Impact:** You correctly identify the affected area (kernel mode of 
sabre-6quad), but you should expand on the other impact points (see below).
   * **Testing:** You provide a solid testing procedure, which is excellent. 
   
   **Areas for Improvement:**
   
   * **Summary:**
       * **Why is the change necessary?** Be more specific about the cause of 
the boot failure. Was it a bug, a configuration issue, etc.?
       * **What functional part of the code is changed?** Mention the specific 
files, modules, or functions you modified.
       * **How does the change work?** Provide a concise explanation of your 
solution. For example, "Fixed an incorrect memory mapping configuration that 
prevented kernel mode initialization."
       * **Related Issues:** If there's a corresponding issue on the NuttX or 
NuttX Apps repositories, link it here. 
   
   * **Impact:**  You need to address all impact points, even if the answer is 
"NO."  Provide brief explanations where relevant. For example:
       * **New feature?** NO 
       * **Existing feature changed?** YES (Kernel mode functionality on 
sabre-6quad)
       * **Impact on user?**  Potentially YES (if they rely on kernel mode). 
Describe any changes users might need to make.
       * **Impact on build?** NO (assuming your changes don't affect the build 
process).
       * **Impact on hardware?** YES (specifically sabre-6quad). 
       * **Impact on documentation?** Maybe YES (if the fix involves 
configuration changes users need to be aware of).
       * **Impact on security?**  Consider if your fix has any security 
implications (positive or negative).
       * **Impact on compatibility?**  Assess potential compatibility issues 
with older NuttX versions or other hardware.
   
   * **Testing:**
       * **Build Host(s):** Specify your development environment (OS, CPU, 
compiler version). 
       * **Target(s):**  You mention the board (sabre-6quad:knsh), but clarify 
the architecture (likely ARM). 
       * **Testing logs:** While you provide the commands, it's essential to 
include the actual *output* of those commands before and after your changes. 
This helps reviewers understand the problem and verify the fix.
   
   **Revised PR Description Template:**
   
   ```markdown
   ## Summary
   
   The sabre-6quad board was failing to boot in kernel mode due to [briefly 
explain the root cause, e.g., an incorrect memory mapping configuration]. This 
PR fixes the issue by [concisely describe your solution, e.g., adjusting the 
memory regions assigned to the kernel during initialization in the 
board-specific configuration file]. 
   
   This change modifies [mention the specific files or modules affected, e.g., 
`boards/arm/armv7-m/sabre-6quad/src/sabre_boot.c`].
   
   * Related Issue: [Link to NuttX issue if applicable]
   
   ## Impact
   
   * Is new feature added? NO
   * Is existing feature changed? YES (Kernel mode functionality on 
sabre-6quad).
   * Impact on user? Potentially YES (if they rely on kernel mode).  [Describe 
any user-facing changes, e.g.,  "Users may need to rebuild their kernel modules 
for kernel mode."]
   * Impact on build? NO
   * Impact on hardware? YES (specifically sabre-6quad).
   * Impact on documentation? [YES/NO] [Explain if documentation needs updates.]
   * Impact on security? [YES/NO] [Explain any security implications.]
   * Impact on compatibility? [YES/NO] [Explain any compatibility concerns.]
   
   ## Testing
   
   I confirm that changes are verified on my local setup and work as intended:
   
   * Build Host(s): [Your OS] ([Your CPU]), [Your compiler] ([compiler version])
   * Target(s): ARM, sabre-6quad:knsh
   
   Testing logs **before** change:
   
   ```
   [Paste the output of your test commands here]
   ```
   
   Testing logs **after** change:
   
   ```
   [Paste the output of your test commands after applying your fix]
   ``` 
   ```
   
   **Remember:** A well-written PR description significantly increases the 
chances of your contribution being reviewed and merged quickly. 
   


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