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

   [**\[Experimental Bot, please feedback 
here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues)
   
   
   
   This PR does **not** fully meet the NuttX requirements.  While it attempts 
to address the summary section, it falls short in several ways:
   
   * **Incomplete Summary:** The summary doesn't clearly explain *why* 
`can_frame` lacking a flag member necessitates changes to `lin.h`.  What 
problem does this solve? What was the original issue?  The explanation of *how* 
the change works is also unclear. Simply moving a flag doesn't explain the 
functional change.  What does this flag represent? How is it used? There's no 
mention of related issues.
   
   * **Missing Impact Assessment:** The impact section is entirely placeholder 
text.  This is critical information.  Even if there's no impact in a particular 
area (e.g., no security impact), explicitly stating "NO" is important.
   
   * **Missing Testing Information:**  The testing section is also placeholder 
text.  There are no details about the testing environment, the tests performed, 
or any logs.  This makes it impossible to assess the validity of the changes.
   
   
   **Here's how to improve the PR to meet the requirements:**
   
   **Summary:**
   
   * **Clearly state the problem:**  Explain why the absence of a flag in 
`can_frame` is an issue. What functionality was missing or broken?  Provide 
context.
   * **Explain the solution in detail:** Don't just say you moved a flag. 
Explain what the flag represents, why it needed to be moved to `lin.h`, and how 
this change addresses the original problem. Provide a conceptual overview of 
the change.
   * **Include related issues:**  If this PR addresses a reported bug or 
implements a requested feature, link the relevant issue(s).
   
   **Impact:**
   
   * **Address each point specifically:**  Go through each item (user impact, 
build impact, hardware impact, etc.) and explicitly state whether there is an 
impact or not.  If there *is* an impact, provide a clear and concise 
description.
   
   **Testing:**
   
   * **Describe your test setup:** Specify the host operating system, CPU 
architecture, compiler version, target architecture, board, and configuration 
used for testing.
   * **Explain your test procedure:** Detail the steps you took to verify the 
changes.  What specific tests did you run? What were you looking for?
   * **Provide before/after logs:** Include relevant log output from both 
before and after applying the changes. This helps demonstrate the effectiveness 
of the fix or the functionality of the new feature.
   
   
   By addressing these points, you will create a much stronger PR that is 
easier to review and more likely to be accepted.
   


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