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

   [**\[Experimental Bot, please feedback 
here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues)
   
   
   
   No. The PR description lacks crucial details required by the NuttX 
guidelines.  While it lists the patches, it doesn't explain *why* these changes 
are necessary or *how* they work.  The impact section is far too brief and 
doesn't address most of the required points (user impact, build impact, 
hardware impact, documentation, security, compatibility).  The testing section, 
while mentioning SIM and CMocka, doesn't provide any actual logs, making it 
impossible to verify the claims.  It also needs to specify the Build Host 
details.
   
   Here's what's missing and needs to be added:
   
   **Summary:**
   
   * **Rationale:** Explain the reasons behind each patch. Why use 
`SEM_INITIALIZER`? Why call init automatically? Why enable dynamic allocation 
by default? What problems do these changes solve or what improvements do they 
provide?
   * **Functionality:** Describe the affected parts of the code in more detail. 
How does the buffer pool mechanism work, and how do these patches modify its 
behavior?
   * **Issue References:**  If applicable, link to related NuttX or NuttX Apps 
issues/PRs.
   
   **Impact:**
   
   * **Feature Changes:**  Clarify if this adds new features or modifies 
existing ones.
   * **User Impact:** Will users need to change their configurations or code to 
use these changes?  If so, how?
   * **Build Impact:**  Will users need to change their build process? Are any 
new dependencies introduced?
   * **Hardware Impact:** Does this affect specific architectures, boards, or 
drivers?
   * **Documentation Impact:** Does this PR require documentation updates? If 
so, are they included?
   * **Security Impact:** Do these changes have any security implications 
(positive or negative)?
   * **Compatibility Impact:**  Are there any backward compatibility concerns?  
Does this affect interoperability with other systems?
   
   **Testing:**
   
   * **Build Host Details:** Provide the OS, CPU architecture, and compiler 
version used for testing.
   * **Target Details:** Specify the simulator and board configurations used 
for testing.
   * **Logs:**  Include *actual* testing logs from both before and after 
applying the patches, demonstrating the change in behavior and verifying that 
the changes work as intended.  Simply stating "CMocka cases" is insufficient.
   
   
   By providing this missing information, the PR will better adhere to the 
NuttX requirements and allow reviewers to effectively evaluate the 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