nuttxpr commented on PR #15443: URL: https://github.com/apache/nuttx/pull/15443#issuecomment-2574493086
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) No, this PR does not fully meet the NuttX requirements. While it provides some information, it lacks crucial details. Here's a breakdown of what's missing: * **Summary:** * While it states the *why* (fixing a compiler error), it's vague about the *what* and *how*. Which files are modified? How does replacing the default `delete` operator fix the issue? What is "size and unsize definition location" supposed to mean? Be explicit. * **Impact:** * The impact section is almost useless. It just repeats part of the summary. Address *all* the impact points specifically. For example: * **Is new feature added? Is existing feature changed?**: No new feature, existing memory deallocation behavior is being fixed. * **Impact on user:** Potentially YES if users rely on specific behavior of the old deallocation, although unlikely. Explain this further. * **Impact on build:** Potentially YES, if users previously suppressed this warning. Explain how. * **Impact on hardware:** Likely NO. Explain why. (e.g., This is a C++ standard library change, no hardware-specific code is modified.) * **Impact on documentation:** Possibly YES if the change affects user code or build process. * **Impact on security:** Possibly YES or NO. Justify your answer. Improper memory management can have security implications. Does this change introduce or mitigate any such issues? * **Impact on compatibility:** Potentially YES if relying on the previous, incorrect behavior. Explain the potential break and what a user would need to do. * **Testing:** * **Insufficient Environment Details:** Provide complete build host details (OS version, compiler version). Specify the target architecture and board configuration you tested on, not just "sim" or "ARM". Be specific (e.g., `qemu-arm`, `stm32f4discovery`). * **Incomplete Logs:** "Compile error" is not a log. Provide the *full* compiler output both before and after the change. Show that the error is gone after the change. Also include runtime tests demonstrating correct behavior. If no runtime change is expected, explain why and what was tested to ensure no regressions. For instance, were existing tests re-run? In short, the PR needs significantly more detail and more thorough testing information to be acceptable. Be explicit and thorough in every section. Don't leave reviewers guessing. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
