nuttxpr commented on PR #14330: URL: https://github.com/apache/nuttx/pull/14330#issuecomment-2413937370
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) Let's break down if this Pull Request (PR) description meets the NuttX requirements. **Areas Needing Improvement** * **Summary:** * **Why is this change necessary?** The summary states what the change *is*, but not *why* it's needed. Is this fixing a bug, improving performance, adding a new feature for gcov support? Clearly articulate the problem being solved. * **What functional part of the code is changed?** Be specific. Mention the affected modules or files (e.g., build system, gcov integration, specific libraries). * **How does it work (at a high level)?** Briefly explain the technical approach. For example: * "The change ensures that the necessary unwind symbol is linked when using Clang 15 for gcov instrumentation." * "A new build option is introduced to..." * **Impact:** While "No" is a start, you must provide justification even if there's no impact. For example: * **Impact on user:** "This change is transparent to end-users as it addresses an internal build/toolchain issue." * **Impact on build:** "This modifies the linking process but doesn't introduce new build steps or dependencies for users." * **...and so on for other impact categories** * **Testing:** * **Insufficient detail:** "NO" is not acceptable. You **must** provide: * **Build Host(s):** List the operating systems, CPUs, and compiler versions you used for testing. * **Target(s):** Specify the architectures (real or simulated) and boards/configurations you tested on. * **Testing Logs:** You need to include actual logs (even if brief) showing the issue before the change and the successful outcome after the change. This provides evidence that your fix works. **Example of an Improved Summary** > **Summary:** This PR addresses a build issue when using Clang 15 for code coverage analysis (gcov) with NuttX. Clang 15's gcov implementation requires the `__aeabi_unwind_cpp_pr0` symbol, which was not being correctly linked. The change introduces a default linking mechanism, similar to `COMPILER_RT_LIB`, to ensure the required symbol is available. Additionally, the `libUnwind` library is updated to call the `__assert_func` API for compatibility. **Key Points** * **Be thorough:** The NuttX project values well-documented changes. Take the time to fill out the PR template completely. * **Show, don't tell:** Provide evidence of your testing. * **Clarity is key:** Make it easy for reviewers to understand your changes and their impact. By improving your PR description, you increase the likelihood of it being reviewed and merged quickly. -- 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
