nuttxpr commented on PR #3062: URL: https://github.com/apache/nuttx-apps/pull/3062#issuecomment-2812938926
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) Yes, this PR appears to meet the NuttX requirements, although some minor improvements could be made. **Strengths:** * **Clear Summary:** The summary explains the change, its purpose, and how it works. * **Impact Assessment:** While concise, it addresses all the points. "None" is acceptable for most if the change is truly isolated. * **Testing:** Includes host/target details and provides *before* and *after* logs demonstrating functionality. **Areas for Improvement:** * **Relate to an issue:** Even if this is a new feature without a prior issue, it's good practice to create an issue first to discuss the idea and get feedback before implementing it. This facilitates community involvement and avoids wasted effort. The PR can then reference the issue. * **More detail on *how* it works:** The summary mentions using the ELF loader to generate a romfs.img. A little more detail here would be helpful. Does this involve new Kconfig options? Changes to the build system? Mentioning specific files modified would make it easier for reviewers. * **Testing - Clarity on *before*:** The "before" logs are unclear. What was the previous behavior? Since this is a new feature, the before might simply be the absence of the feature, but it's worth explicitly stating that. Or perhaps the "before" is a system without the external romfs, and you're showing that the elf loader still works as expected for internal romfs. **Conclusion:** The PR is well-structured and provides the necessary information. Adding an associated issue and slightly expanding the technical details and "before" test explanation would strengthen it further. -- 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
