nuttxpr commented on PR #13568: URL: https://github.com/apache/nuttx/pull/13568#issuecomment-2367074470
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) The PR does not meet the NuttX requirements fully. **Here's why and how to improve it:** * **Summary:** * While it mentions the "what" (reverting a deadlock assert), it lacks a clear explanation of **why** the original change is being reverted. Was it incorrect, redundant, causing issues? * Be explicit about the functional area – is it just the pthread mutex implementation? * **Impact:** * "N/A" is rarely fully accurate. Even a revert can have subtle impacts. Consider: * **Compatibility:** Did the reverted change introduce any behavior that, while potentially buggy, some code might have relied on? * **Performance:** Could removing the assert, even if redundant, have a minor performance impact (likely negligible, but worth mentioning if measurable)? * **Testing:** * "ci-check" is not sufficient. While CI is essential, a PR needs evidence of manual testing relevant to the change: * **What specific tests were run?** Ideally, provide commands or test names. * **Logs are missing.** Include both "before" and "after" logs to demonstrate the change's effect. **In short:** The PR needs more context and explicit testing details to be considered complete against the NuttX requirements. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
