nuttxpr commented on PR #15522: URL: https://github.com/apache/nuttx/pull/15522#issuecomment-2588605589
[**\[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 and testing information, it's lacking crucial details. Here's a breakdown of what's missing: * **Insufficient Summary:** While it states the *what* (reverting a specific commit), it lacks the *why*. Why is the original change being reverted? What specific problem did it cause? What are the benefits of reverting? How does using the original `enter_critical_section` improve the situation (i.e., how does it stop other CPUs)? * **Impact Section Too Brief:** "N/A" is not acceptable. Even a revert can have impact. At minimum, the impact on security (re-introducing a potential vulnerability?) and compatibility should be addressed. There might also be performance implications. * **Lack of Detail in Testing:** "ci-check" is insufficient. What specific tests were run? What platforms? What were the observed results *before* and *after* the revert? Actual log output or a link to CI results should be included. The current testing section provides no evidence the change actually works as intended. To summarize: The PR needs to explain the rationale behind the revert, analyze its impact in more detail, and provide concrete evidence from testing to support the claim that the change is beneficial. -- 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]
