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

   [**\[Experimental Bot, please feedback 
here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues)
   
   
   
   This PR appears to be missing key information required by the NuttX 
guidelines.  While it provides a basic summary of the changes (merging gcov 
scripts and implementing in Python), it lacks crucial details in the Impact and 
Testing sections.
   
   **Missing Information & How to Fix It:**
   
   * **Summary:** Although it mentions what changed, it doesn't explain *why*. 
What problem did this solve?  Was the shell script problematic?  Was this a 
performance improvement?  This section needs to justify the change.  Also, link 
a related NuttX issue if one exists.
   
   * **Impact:**  This section is completely empty.  The author *must* address 
each point:
       * **Is new feature added? Is existing feature changed?**:  Clarify this. 
Sounds like an existing feature (gcov support) was refactored.
       * **Impact on user:** Will users need to do anything differently to use 
gcov?
       * **Impact on build:**  Will the build process change (e.g., new 
dependencies)?
       * **Impact on hardware:**  Likely NO, but explicitly state it.
       * **Impact on documentation:** Does the documentation need updating to 
reflect these changes?  If so, has the documentation been updated in this PR or 
a separate one?
       * **Impact on security:**  Likely NO, but explicitly state it.
       * **Impact on compatibility:** Are there any backward compatibility 
concerns? Will older gcov data still work?
       * **Anything else to consider?**: Anything unexpected or noteworthy?
   
   * **Testing:** This section is also empty.  The author *must* provide:
       * **Build Host(s):**  Full details: OS, CPU architecture, compiler 
version. Example:  "Ubuntu 20.04, x86_64, GCC 9.4.0"
       * **Target(s):**  Architecture and board/configuration tested on. 
Example: "sim:qemu-x86_64", "stm32f4discovery:nsh"
       * **Testing logs before change:**  Show the output of gcov *before* the 
change.
       * **Testing logs after change:** Show the output of gcov *after* the 
change, demonstrating that it still works (or works better, if that was the 
goal).
   
   **Conclusion:**
   
   This PR does **not** meet the NuttX requirements in its current state.  The 
author needs to provide significantly more detail in the Impact and Testing 
sections, and clarify the reasoning behind the change in the Summary section.  
Without this information, it's impossible to properly review the PR and assess 
its suitability for merging.
   


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