nuttxpr commented on PR #15075: URL: https://github.com/apache/nuttx/pull/15075#issuecomment-2523088664
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) No. The PR description lacks several key details required by the NuttX guidelines. Here's a breakdown: * **Insufficient Summary:** While the summary mentions *what* changed, it lacks crucial *why* information. Why was this change necessary? What problem does it solve? What are the benefits of using atomic operations and avoiding the critical section? * **Missing Impact Details:** Simply stating "semaphore" is insufficient. The impact section needs specific YES/NO answers for *all* listed questions. For any "YES" answers, a detailed explanation is required. For example, even if the user impact is NO, explicitly state "Impact on user: NO." Consider these points for your impact assessment: * **Performance:** The summary mentions performance improvements. This should be explicitly stated in the Impact section, quantifying the improvement. * **Behavior Change:** Does this change the behavior of semaphores in any way beyond performance? Even subtle changes can affect existing applications. * **Portability:** Does this change affect portability to other architectures? Does it rely on specific atomic operations that might not be available everywhere? * **Incomplete Testing Information:** While testing environments are mentioned, it lacks details required by the template. The provided information is too vague. * **Build Host:** Specify OS version, compiler version (e.g., GCC 12.2), etc. * **Target:** Be specific. Instead of "sim," specify the simulator being used (e.g., qemu-system-arm). For boards, specify the configuration used (e.g., `sabre-6quad:smp`). * **Missing "Before" and "After" Logs:** The provided timing information is helpful, but insufficient. The template specifically requests *logs*, not just summarized timings. Show relevant log excerpts demonstrating the semaphore behavior before and after the change. Include more than just the timings - show the context of the semaphore usage in the test. * **Missing Issue References:** Are there any related NuttX or NuttX Apps issues that prompted this change? If so, include links. **In short:** The PR needs to be significantly expanded to address these missing details before it can be considered complete according to the NuttX requirements. Focus on providing specific, detailed answers to all questions in the template. -- 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
