nuttxpr commented on PR #14810: URL: https://github.com/apache/nuttx/pull/14810#issuecomment-2478357755
[**\[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 testing information, it lacks crucial details. Here's what's missing: * **Summary:** While it states *what* is being changed, it doesn't fully explain *why*. What are the benefits of removing `syslog_putc()` besides "simplifying"? Does it improve performance, reduce code size, or fix a bug? This is the most important part of the summary and needs to be strengthened. It also needs links to related issues if applicable. * **Impact:** Simply stating "N/A" is insufficient. Each impact item needs to be explicitly addressed with either "NO" or "YES (with explanation)". Even if there's no impact, stating "NO" for each item demonstrates that the impact has been considered. For example, are you *sure* there's no impact on compatibility? Could code relying on `syslog_putc()` break? * **Testing:** "ci-check" is not sufficient. While CI is important, the requirements specifically request testing logs *before* and *after* the change. These logs should demonstrate that the change works as intended and doesn't introduce regressions. What specific tests were run? Provide concrete examples, even if simple. Also, specify the build host and target details as requested. Therefore, while the PR provides a starting point, it needs more information before it can be considered complete according to the provided guidelines. -- 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: commits-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
