nuttxpr commented on PR #3060: URL: https://github.com/apache/nuttx-apps/pull/3060#issuecomment-2808682492
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) Yes, this PR appears to meet the basic NuttX requirements, but it lacks crucial details. While it addresses the "why" (using the macro for consistency and to avoid conflicts), it's missing important information: * **Insufficient Summary:** What functional part of the code uses this semaphore? Be specific (e.g., "in the I2C driver initialization"). How *exactly* does the change work (beyond just "using the macro")? Does it change the semaphore's initial count or behavior? * **Missing Testing Details:** "Tested that this compiles" is not enough. What target architecture and board did you test on? What functionality depends on this semaphore, and did you test that functionality before and after the change? Provide actual logs or test output, even if it's simple. * **Impact Section Too Brief:** While the impact is likely minimal, stating "No impact" for all categories without justification isn't best practice. Briefly explain *why* there's no impact on each, especially documentation. If documentation doesn't need an update, say so explicitly. For example: "Impact on documentation: NO - This is a purely internal code style change and does not affect user-facing behavior." **In short, while the PR is on the right track, it needs more specific information and more thorough testing details to be properly reviewed and merged.** -- 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
