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

   [**\[Experimental Bot, please feedback 
here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13494&type=pullrequests)
   
   The PR summary is lacking some key information, and the testing section is 
inadequate. Here's a breakdown:
   
   **What's Missing/Needs Improvement:**
   
   * **Insufficient Detail in Summary:**
       * **What functional part?** Be specific. Is it the uORB messaging layer 
itself? A specific driver? 
       * **How does the fix work?** Briefly explain the technical approach 
(e.g., "The fix ensures the stublist is updated by...").
   * **Impact:**
       * **New feature? Existing feature changed?**  A bug fix usually means an 
existing feature is fixed, but clarify this.
       * **Impact on user?** This bug fix likely impacts users relying on 
cross-core sensor communication. Explain how their experience might have been 
affected by the bug and how this fix resolves it.
       * **Impact on documentation?**  If the bug fix reveals a 
misunderstanding in how the system works, documentation might need updating.
   * **Testing:**
       * **Incomplete Environment:** List the *specific* versions of OS, 
compiler, target architecture (e.g.,  "Linux Ubuntu 20.04, GCC 9.4.0, 
SIM:qemu-system-arm").
       * **No "Before" Logs:** Without logs demonstrating the bug, it's 
impossible to verify the fix.
       * **Unclear "After" Logs:** The provided logs are hard to follow.  
Provide clear steps to reproduce the issue and show how the logs now 
demonstrate the correct behavior after the fix.
   
   **Revised Testing Section (Example):**
   
   ```
   ## Testing
   
   I confirm that changes are verified on the following setup:
   
   * **Build Host:** Ubuntu 20.04, GCC 9.4.0
   * **Target:** SIM: qemu-system-arm (configs: rpserver, rpproxy)
   
   **Steps to Reproduce:**
   
   1. **Before the fix:**
      - Start rpserver: `./nuttx/tools/nsh.sh sim:configs/rpserver`
      - Run: `hello &` and `hello 1 &`
      - Start rpproxy: `./nuttx/tools/nsh.sh sim:configs/rpproxy`
      - Run: `hello 1 1 &`
      - Observe the logs on both rpserver and rpproxy. The bug manifests as 
[describe the incorrect behavior, e.g., messages not being received, unexpected 
message overwrites]. 
   
   2. **After the fix:**
      - Repeat steps 1-4.
      - Observe the logs. The fix ensures [describe the expected correct 
behavior, e.g., messages are now correctly received and processed without 
overwrites].
   
   **Logs:**
   
   * **Before Fix (trimmed for brevity - attach full logs):**
     ```
     [rpserver logs showing the incorrect behavior]
     [rpproxy logs showing the incorrect behavior] 
     ```
   
   * **After Fix (trimmed for brevity - attach full logs):**
     ```
     [rpserver logs showing the correct behavior]
     [rpproxy logs showing the correct behavior] 
     ``` 
   ```
   
   **Additional Tips:**
   
   * **Conciseness:** Aim for a clear and concise PR description. Avoid overly 
technical jargon unless necessary.
   * **Focus:** Stick to the essential details of the change.
   * **Completeness:** Don't leave out crucial information that reviewers need 
to understand and test your changes. 
   


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