nuttxpr commented on PR #2882:
URL: https://github.com/apache/nuttx-apps/pull/2882#issuecomment-2527088192

   [**\[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.
   
   * **Summary:** While it describes the change, it lacks crucial information 
like *why* this change is necessary. Is there a problem with the current 
logging method? Does this improve performance, debugging, or user experience?
   * **Impact:**  "N/A" is insufficient.  While the impact *might* be minimal, 
you must explicitly state that and justify it.  For instance, does this change 
the memory usage? Does it add any new dependencies?  Consider each point 
specifically.  If the answer is truly "NO" for each, then state "NO" and 
briefly explain why (e.g., "Impact on build: NO, only adds a config option").
   * **Testing:** The testing logs don't show a comparison.  You need logs 
*before* the change using the old logging method, and logs *after* the change 
demonstrating the redirection to syslog.  Simply providing the steps to build 
and run the test isn't enough.  Show the actual output difference. Also, 
specify the details of the `sim/tflm` target.  What version of NuttX are you 
using? What specific simulator is this (QEMU?)?
   
   The author needs to revise the PR to address these shortcomings before it 
can be properly reviewed.
   


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