nuttxpr commented on PR #15159: URL: https://github.com/apache/nuttx/pull/15159#issuecomment-2538711728
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) The PR mostly meets the NuttX requirements, but is missing some important details. Here's a breakdown: **Strengths:** * **Clear Summary:** The summary explains the "why," "what," and "how" of the changes. * **Impact Section Addresses Key Areas:** It acknowledges the impact on users and memory consumption. * **Testing Mentions Platform:** It specifies the target platform (MPFS board) and build type (flat-mode). **Weaknesses & Missing Information:** * **Missing Issue References:** No related NuttX or NuttX Apps issue/PR is linked. Even if there isn't one, stating "N/A" is good practice. * **Impact Section Incomplete:** Several "Impact" fields are missing: * **Impact on build:** While it mentions "flat-mode build," it doesn't explain *how* the build process is affected by the changes. Does anything need to be reconfigured? * **Impact on hardware:** Specify which architectures are impacted. Even if it's "all," state it explicitly. * **Impact on documentation:** Does the documentation need updating to reflect these changes? * **Impact on security, compatibility:** Even if the answer is "NO," it should be explicitly stated for each. * **Testing Insufficient:** * **Build Host Details Missing:** Provide details about the build host OS, CPU, and compiler version. * **Target Details Insufficient:** "MPFS board" is not specific enough. Provide the exact board configuration used. * **No Logs:** The "Testing logs before/after change" sections are empty. While the description mentions observing behavior with "several hundred shm objects," concrete logs demonstrating the problem *before* the change and the fix *after* the change are crucial. Show how the zero-initialization was broken and how memory consumption changed. Quantitative data is ideal. **Recommendations for Improvement:** 1. **Add Issue References:** If no related issues exist, state "N/A." 2. **Complete Impact Section:** Fill in all fields, even with "NO" if applicable. Justify "YES" answers with details. Example for build impact: "No changes to the build process itself, but applications using SHM in flat-mode builds might experience different behavior due to the zero-initialization fix." 3. **Provide Detailed Testing Information:** * Specify build host details (OS, CPU, compiler). * Specify the exact target board and configuration. * Include *actual* logs demonstrating the issue before the change and the successful fix after the change. Include memory usage data or other quantifiable metrics to show the impact. By addressing these points, the PR will be significantly stronger and adhere more closely to the NuttX requirements. This makes review easier and increases the chances of acceptance. -- 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]
