nuttxpr commented on PR #16241: URL: https://github.com/apache/nuttx/pull/16241#issuecomment-2817769121
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) Yes, this PR appears to meet the basic NuttX requirements, but it lacks crucial details. While it addresses the sections required, the content is too brief and lacks specifics. Here's why it's insufficient and what needs improvement: * **Summary:** While it mentions preserving `CONFIG_ARCH_BOARD_COMMON` and links a related issue, it doesn't explain *why* this change is necessary. What problem does issue #12122 describe? How does preserving this config setting fix that problem? What part of the `make savedconfig` tool is changed? * **Impact:** Saying it "fixes configurations" isn't enough. Be explicit. Does this affect how users configure their boards? Does it change the build process itself? Are any specific architectures, boards, or drivers affected? Does this change require documentation updates? Are there any security implications to losing this setting (which is what the bug presumably caused)? Even if the answer to most of these is "NO", explicitly stating "NO" with brief justification is better than leaving them blank or vague. * **Testing:** "Tested with out of tree board config" is far too vague. Provide specific details: * **Build Host:** What operating system, CPU architecture, and compiler version did you use for building? * **Target:** What architecture and board configuration did you test on? Was this a simulator (`qemu`) or real hardware? * **Logs:** The "Testing logs before change" and "Testing logs after change" sections are empty. Include actual logs demonstrating the problem before the change and the corrected behavior after the change. Showing the *difference* the change makes is crucial. Even simple logs like the output of `make savedconfig` and the contents of the resulting `.config` file would be much better than nothing. **In short:** While the PR touches on the required sections, it's missing the essential details that allow reviewers to understand the change, its impact, and its verification. More complete and specific information is needed for proper review and acceptance. -- 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
