nuttxpr commented on PR #2677:
URL: https://github.com/apache/nuttx-apps/pull/2677#issuecomment-2406445065

   [**\[Experimental Bot, please feedback 
here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues)
   
   
   
   **No, this PR does not meet the NuttX requirements.**
   
   **Missing Information:**
   
   * **Summary:**
       *  **Why** is the change from `ack` to `cmd` necessary? What problem 
does it solve? 
       * **How** does the change to `cmd` work? What are the possible values of 
`cmd` and what do they signify? 
   * **Impact:**
       * The impact section is too vague. Be specific about which features are 
impacted. 
       * **User Impact:**  Will users need to change their applications due to 
the renaming from `ack` to `cmd`?
       * **Documentation:** Does the documentation need to be updated to 
reflect the change from `ack` to `cmd`?
   * **Testing:**
       * **Insufficient Detail:** "sim rpserver and rpproxy" is not enough 
information. Specify the simulator used (e.g., QEMU), the architecture (e.g., 
ARMv7-M), and the NuttX configuration.
       * **No Logs:**  Provide actual testing logs before and after the change. 
This helps demonstrate the problem being solved and that the change works as 
intended.
   
   **Recommendations:**
   
   1. **Elaborate on the "Why" and "How":** Explain the rationale for renaming 
`ack` to `cmd`. Provide details about the new structure and how the `cmd` field 
is used.
   2. **Specify Impact:**  Clearly state which features are affected and how. 
For example, "This change affects the rpmsg ping example application. Users 
will need to update their applications to use 'cmd' instead of 'ack'."
   3. **Provide Detailed Testing Information:** Specify the simulator, 
architecture, NuttX configuration, and include actual log output demonstrating 
the change's effectiveness. 
   
   By providing this missing information, you can significantly improve the 
quality of your PR and make it easier for reviewers to understand and accept 
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