nuttxpr commented on PR #2769: URL: https://github.com/apache/nuttx-apps/pull/2769#issuecomment-2436635820
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) The PR summary you provided lacks some key details to determine if it meets the NuttX requirements. Here's a breakdown and what needs improvement: **Strengths:** * **Clear problem statements:** Both points in the summary clearly identify the issue being addressed. * **Impact is addressed:** It broadly identifies affected boards. **Weaknesses:** * **Missing details about the change itself:** * The summary only describes the *problem* not the *solution*. How exactly are you disabling ICANON in termcurses? What code is modified? * How is `release_vi` being incorporated to fix the error condition exit? * **Impact section is too vague:** * "All VI-enabled boards" is not specific enough. Which architectures? Are there known boards where this *won't* be an issue? * The other impact categories are completely skipped. At minimum, address: * **Build:** Did the build process change at all due to your modifications? * **Documentation:** Does this fix require any documentation updates? * **Testing is insufficient:** * **Only one host/target combination:** NuttX supports a wide range of platforms. Testing on a single simulated environment is not enough. * **No logs provided:** The "Testing logs" sections are empty, making it impossible to verify the fix. **To meet NuttX requirements, the PR needs these improvements:** 1. **Elaborate on the solution:** Don't just describe the problem. Explain the code changes made to implement the fix. 2. **Be specific about impact:** List affected architectures and, if possible, any boards known to be *unaffected*. Address all other impact categories. 3. **Expand testing:** * Test on at least one more significantly different target (e.g., a real board if you initially tested on simulator). * **Include relevant logs:** Show output *before* and *after* your change to demonstrate the fix. 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
