nuttxpr commented on PR #14926: URL: https://github.com/apache/nuttx/pull/14926#issuecomment-2497216797
[**\[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 of the change itself, it lacks crucial information. Here's a breakdown: * **Insufficient Summary:** While the summary describes *what* was changed, it doesn't explain *why*. What problem did the recursive calls cause? What functionality was affected? Were there any bug reports (NuttX issue references) related to this? * **Missing Impact Details:** Simply writing "note" under Impact is inadequate. Each impact category (user, build, hardware, documentation, security, compatibility) needs a YES/NO answer followed by a description if the answer is YES. Even if the answer is NO for all, it should be explicitly stated (e.g., "Impact on user: NO"). Consider the implications of removing the critical section. Could this lead to race conditions? Does it affect interrupt handling? * **Inadequate Testing:** "ci ostest" isn't sufficient. What specific tests were run? Provide *actual* log output before and after the change. What build host was used? What target architecture and board configuration? The logs should demonstrate that the problem existed before the change and is resolved after the change. Just saying "ci ostest" doesn't prove anything without the supporting details and logs. In short, the PR needs to be significantly more detailed to meet the requirements. It needs to provide the missing context and evidence to justify the change and demonstrate its effectiveness. -- 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
