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

   [**\[Experimental Bot, please feedback 
here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues)
   
   
   
   No, this PR does not fully meet the NuttX requirements.  While it provides a 
summary and mentions testing, it lacks crucial details required for a proper 
review.
   
   Here's a breakdown of what's missing:
   
   * **Insufficient Summary Detail:** While the summary mentions adding a 
`get_info` function, it doesn't explain *why* this change is necessary.  Is it 
for a new feature, bug fix, or something else? What kind of "basic information" 
is being retrieved?  How does the function work?  Are there related NuttX 
issues?
   
   * **Incomplete Impact Assessment:**  Simply stating "Now goldfish sensor can 
get sensor basic information" is not enough.  The PR needs to specifically 
address *all* the impact points:
       * **Is this a new feature?**  (Presumably YES)
       * **User Impact:** Will users need to change their code to use this new 
function?
       * **Build Impact:**  Are there any changes to the build system, Kconfig 
options, etc.?
       * **Hardware Impact:** Does this only affect the goldfish simulator, or 
real hardware as well?  Which specific architectures/boards are affected?
       * **Documentation Impact:**  Does the documentation need to be updated 
to describe this new function?  Has the documentation been updated in this PR?
       * **Security Impact:**  Are there any potential security implications of 
adding this function?
       * **Compatibility Impact:**  Does this change break any existing 
functionality or introduce compatibility issues?
   
   * **Inadequate Testing Information:** "goldfish uorb_listener" is not 
sufficient testing information.  The PR needs to specify:
       * **Build Host Details:** OS, CPU architecture, compiler version used 
for building NuttX.
       * **Target Details:**  Specific architecture, board, and configuration 
used for testing.
       * **Comparative Logs:**  "Testing logs before change" and "Testing logs 
after change" sections should contain actual logs demonstrating the 
functionality before and after the change.  Simply stating "uorb_listener" 
doesn't show *how* the change works or that it fixes a problem.  Show the 
output of `uorb_listener` before and after the change, highlighting the new 
information provided by `get_info`.
   
   The PR author needs to provide significantly more detail in all these 
sections to meet the NuttX requirements.
   


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