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

   [**\[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 provides a 
summary of the changes, it lacks crucial information in the Impact and Testing 
sections.
   
   Here's a breakdown:
   
   * **Summary:**  Adequate, but could be improved by explicitly stating *why* 
enabling LTO by default for arm64/qemu is desirable and *why* it needs to be 
turned off by default for kasan/lto.  What problem does this solve? What are 
the benefits?  Clarifying the incorrect conditional and the proposed correction 
is good.
   
   * **Impact:** Completely inadequate.  This section needs to be filled out.  
Consider the following:
       * **Impact on user:** Will user behavior change? Will they need to 
reconfigure anything? Will performance be affected?
       * **Impact on build:**  Will build times change? Will disk space usage 
change?  Will any new dependencies be introduced?
       * **Impact on hardware:**  Does this change affect specific hardware 
configurations?
       * **Impact on documentation:** Does the documentation need to be updated 
to reflect this change?
       * **Impact on security:**  Are there any security implications of 
enabling/disabling LTO?
       * **Impact on compatibility:** Does this break compatibility with any 
existing configurations or tools?
   
   * **Testing:** Completely inadequate. This section must be filled out with 
specific details:
       * **Build Host(s):**  Provide the OS, CPU architecture, and compiler 
version used for building.
       * **Target(s):** Specify the architecture, board, and configuration used 
for testing.
       * **Testing logs before change:** Include relevant logs demonstrating 
the behavior *before* the change. This could include build output, runtime 
output, or performance benchmarks.
       * **Testing logs after change:** Include corresponding logs 
demonstrating the behavior *after* the change, showing how the issue was 
resolved or the new feature functions.
   
   
   **In short:** The PR needs significant expansion in the Impact and Testing 
sections before it can be properly reviewed.  Simply stating "Update this 
section..." is insufficient.  Provide concrete details and evidence to 
demonstrate the change's validity and impact.
   


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