nuttxpr commented on PR #16148: URL: https://github.com/apache/nuttx/pull/16148#issuecomment-2782059766
[**\[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's missing some crucial information. **Missing Information & Suggestions:** * **Summary:** While it describes the *what*, it lacks the *why*. Why is setting the mutex flag important even without immediate functional impact? What future optimizations are enabled by this? Why was direct access to `semcount` problematic and why is `nxsem_get_value` preferred? Be explicit. * **Impact:** While stating "no functional impact" is good, it needs more detail regarding the other impact categories. Explicitly state "NO" for each category (user, build, hardware, documentation, security, compatibility) or explain why it's a "YES". Even if the answer is "NO," briefly stating why reinforces that these areas were considered. For instance: "Impact on build: NO (no changes to the build system are required)." * **Testing:** The listed targets are good, but the provided test logs are placeholders. Provide *actual* logs showing the behavior before and after the change. Even if there's no functional change, demonstrate that the code behaves as expected. For example, show output of a test program using pthreads and semaphores. The more evidence you can provide, the easier it is for reviewers to assess the impact and correctness of the changes. Consider adding a simple test case demonstrating the use of the `nxsem_get_value` interface if applicable. In short, the PR provides a starting point, but it needs more detail and actual test results to meet the full requirements for review and merging. -- 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
