nuttxpr commented on PR #15345: URL: https://github.com/apache/nuttx/pull/15345#issuecomment-2562134672
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) No, this PR description does not fully meet the NuttX requirements. While it provides a summary and testing information, it lacks crucial details required for proper review. Here's a breakdown of what's missing and how to improve it: **Missing Information & Suggested Improvements:** * **Summary:** * **Why is this change necessary?** The summary states *what* the change does, but not *why*. Is `nxsig_deliver` causing a performance bottleneck? Is it a maintainability issue? Explain the motivation for this change. * **What functional part of the code is being changed?** Be more specific than "tcb". Which files are modified? Mentioning specific modules or subsystems within NuttX would be helpful. * **How *exactly* does the change work?** The summary is a bit vague. Describe the mechanism of using a flag bit. What flag bit is used? How does the logic change for signal delivery with this new flag? * **Related Issues:** Are there any related NuttX or NuttX Apps issues? If not, explicitly state "None." * **Impact:** While stating "Code modification is imperceptible to users" is a good start, you need to specifically address *all* the impact points with either "NO" or "YES (and explanation)": * **New/Changed Feature:** NO * **Impact on User:** NO * **Impact on Build:** Likely NO, but confirm. If there are any changes to Kconfig options or build dependencies, explain them. * **Impact on Hardware:** Specify the architectures tested (armv7a, armv8a). Were any specific boards used? State YES and list them if applicable. Otherwise, if it's architecture-generic, explain why. * **Impact on Documentation:** NO (unless documentation needs to be updated to reflect the internal code changes, in which case say YES and provide details). * **Impact on Security:** Carefully consider this! Does changing the signal handling mechanism introduce any potential security vulnerabilities? Justify your answer (NO/YES). * **Impact on Compatibility:** NO (presumably, but explain why you think so). Does this change affect any existing drivers or applications? * **Anything else to consider:** Mention any potential risks or edge cases that reviewers should be aware of. * **Testing:** * **Build Host(s):** Provide details about your build host environment: OS, CPU architecture, compiler version (e.g., "Linux x86_64, GCC 12.2.0"). * **Target(s):** Be more specific: "sim:qemu-system-arm", "board:stm32f4discovery:default", etc. * **Testing Logs:** The provided logs are placeholders. Include *actual* logs or output demonstrating the test results before and after the change. If the output is too long, consider attaching a file or linking to a CI run. Show that the change works as expected and doesn't introduce regressions. What specific aspects of `ostest` were tested? By addressing these points, you'll create a much more comprehensive and reviewable PR, increasing its chances of being accepted. Remember to follow the contributing guidelines carefully. -- 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
