nuttxpr commented on PR #15878: URL: https://github.com/apache/nuttx/pull/15878#issuecomment-2671062212
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) This PR description mostly meets the NuttX requirements, but could be improved. Here's a breakdown: **Strengths:** * **Clear explanation of the problem:** The summary clearly describes the race condition and the unexpected interrupt behavior. * **Detailed solution:** The "how" is well-articulated, outlining the steps taken to address the pending interrupt issue. * **Impact is addressed:** The impact section specifies the affected targets and the benefits of the change. * **Testing information is provided:** While not showing actual logs, it at least mentions the testing environments. **Weaknesses:** * **Missing Issue References:** The summary should link to a related NuttX issue if one exists. If not, consider creating one before submitting the PR. This helps with tracking and provides context. * **Impact Section could be more structured:** While the general impact is covered, using the provided "NO/YES" structure would improve clarity. For example: * Impact on user: NO * Impact on build: NO * Impact on hardware: YES (Affects MPFS targets) * Impact on documentation: NO/YES (depending on if docs need updating. If yes, describe) * Impact on security: NO/YES (If there were security implications before, then YES and explain the improvement) * Impact on compatibility: NO/YES (Justify the answer) * **Missing Testing Logs:** "Desk-tested" is vague. Provide actual logs demonstrating the issue before the change and the corrected behavior after the change. This is crucial for verifying the fix. Even simple `printf` statements can be helpful. * **"Anything else to consider?" is omitted:** While not always necessary, it's good practice to include this section to preemptively address potential concerns or further development. **Suggested Improvements:** 1. **Create/Link a NuttX issue.** 2. **Restructure the Impact section using the NO/YES format.** For each YES, provide a concise explanation. 3. **Replace "Desk-tested" with concrete testing logs.** Show output demonstrating the problem before the change and the improved behavior after. 4. **Add the "Anything else to consider?" section,** even if it's just "None." By making these changes, the PR will be more comprehensive, easier to review, and more likely to be accepted. -- 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
