nuttxpr commented on PR #3065: URL: https://github.com/apache/nuttx-apps/pull/3065#issuecomment-2833552859
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) __Fill In The Commit Message:__ This PR contains a Commit with an Empty Commit Message. Please fill in the Commit Message with the PR Summary. No, this PR likely does *not* fully meet the NuttX requirements. While it provides a basic summary and impact assessment, it lacks crucial details: * **Insufficient Summary:** While the reasoning for the change is provided, it doesn't explain *how* the "hello world" example works or interacts with NuttX specifics. It needs to describe what the example demonstrates (e.g., POSIX APIs, task creation, console output). * **Missing Technical Detail in Impact:** Saying "Just adding a new example" is not enough. Does it use any specific NuttX features? Does it depend on any particular configurations? While it *might* not change the build process or hardware support directly, this needs to be explicitly stated (NO) and justified if applicable. Documentation impact is also unclear – does this example require documentation? * **Inadequate Testing:** The testing section is severely deficient. It lists the environments, but provides *no actual test logs*. What was the output before and after the change? How do you know it "works as intended"? "Works as intended" is subjective; provide concrete evidence. Just stating the OS and kernel version isn't sufficient; mention the compiler used for both NuttX and Linux builds. For the NuttX target, specifying just `sim` is too broad; specify the simulator configuration used (e.g., `sim:nsh`). **To improve this PR:** 1. **Expand the Summary:** Explain the example's functionality, the POSIX APIs it uses, and how it interacts with NuttX (e.g., task startup, standard output). 2. **Clarify Impact:** Explicitly state "NO" for aspects like build process changes, hardware impact, security implications, etc., along with a brief justification (e.g., "NO - This is a self-contained example that doesn't modify existing NuttX code or hardware drivers"). If documentation is needed, state "YES" and describe what documentation is required or provided. 3. **Provide Detailed Testing Logs:** Show the console output *before* and *after* applying the change, both for the NuttX simulator and the Linux build. This proves that the example compiles, runs, and produces the expected "hello world" output in both environments. Be explicit about the compiler used for both builds. Use a specific simulator configuration (e.g., `sim:nsh`). Example: ``` Testing logs before change (NuttX sim:nsh): (No prior example existed) Testing logs after change (NuttX sim:nsh): nsh> hello_nuttx Hello, NuttX World! nsh> Testing logs before change (Linux, GCC 12.2.0): (No prior example existed) Testing logs after change (Linux, GCC 12.2.0): $ ./hello_linux Hello, Linux World! $ ``` By addressing these points, the PR will be much more likely to be accepted. Remember, clear and thorough information is essential for reviewers to understand and evaluate the changes. -- 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
