nuttxpr commented on PR #16214: URL: https://github.com/apache/nuttx/pull/16214#issuecomment-2804651173
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) Yes, this PR appears to mostly meet the NuttX requirements, but it's missing some crucial information. Here's a breakdown of what's good and what needs improvement: **Strengths:** * **Clear problem statement:** The summary clearly identifies the issue (broken IRQ prioritization) and the root cause (incorrect register addresses). * **Impact on user is addressed:** The impact is stated as "Working IRQ prioritize on rp23xx," which implies a positive change for users on that platform. * **Configuration option mentioned:** Referencing `CONFIG_ARCH_IRQPRIO=y` is helpful for understanding the context. **Weaknesses:** * **Incomplete Summary:** While the problem is stated, the "how it works" part of the summary is missing. *How* were the register addresses fixed? What were the incorrect values and what are the correct ones? * **Missing Issue References:** Are there any related NuttX or NuttX Apps issues? Even if there aren't, explicitly stating "None" is helpful. * **Impact Section is Severely Underdeveloped:** Almost all "Impact" questions are unanswered. This is the most significant area for improvement. At a minimum, address the impact on: * **Hardware:** Specifically mention rp23xx (and any other affected architectures). * **Documentation:** Does this change require documentation updates? * **Compatibility:** Are there any backward compatibility concerns? * **Missing Testing Details:** The "Testing" section is entirely empty. This is critical. Provide: * **Build Host Details:** OS, CPU architecture, compiler version. * **Target Details:** Specific rp23xx board and configuration used for testing. * **Testing Logs:** "Before" and "after" logs demonstrating the fix. Even simple logs showing the IRQ prioritization working (or not working before the fix) would be valuable. **How to Improve the PR:** 1. **Expand the Summary:** Explain the *how* of the fix. 2. **Add Issue References:** If none exist, state "None." 3. **Complete the Impact Section:** Address all the questions, even if the answer is "NO." Be explicit. 4. **Provide Detailed Testing Information:** Fill in the Build Host(s), Target(s), and crucially, the testing logs. Show evidence that the fix works. By addressing these points, the PR will be much stronger 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
