nuttxpr commented on PR #15110: URL: https://github.com/apache/nuttx/pull/15110#issuecomment-2531357363
[**\[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 reason for the change, it lacks crucial details. Here's a breakdown of missing information: * **Summary:** * Missing: "What functional part of the code is being changed?" (e.g., "The scheduler," "Interrupt handling," specific drivers/files). * Missing: "How does the change exactly work?" (e.g., "Replaced all instances of `spin_lock_irqsave(NULL)` with `spin_lock()` because...", "Refactored the interrupt handling logic to avoid the need for...", etc.). * Missing: Issue/PR references. * **Impact:** * "RELEASE" is insufficient. Address *all* impact points with "YES" or "NO" and provide descriptions where necessary. Even if the answer is "NO," explicitly stating it helps reviewers. For a change affecting spinlocks, there might be performance implications. * Consider: Are there performance changes? What about code size changes? * **Testing:** * "CI" is not sufficient. While CI is important, the PR needs to show *your* local testing results. * Missing: Build host details (OS, CPU, compiler). * Missing: Target details (architecture, board, configuration). * Missing: Actual testing logs *before* and *after* the change. Just saying "CI" doesn't show what was tested or the results. In short, the PR needs significantly more detail to meet the NuttX requirements. Reviewers need enough information to understand the change, its impact, and how it was tested to properly evaluate it. -- 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]
