nuttxpr commented on PR #14286: URL: https://github.com/apache/nuttx/pull/14286#issuecomment-2412896457
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) This PR appears to meet the basic NuttX requirements, but could use more details for a comprehensive review. Here's a breakdown: **Strengths:** * **Clear Summary:** You've clearly stated the "why" (duplicate functions) and "what" (removing one implementation). * **Testing Done:** You've indicated testing on a simulator. **Areas for Improvement:** * **Summary - How:** Be more specific about which implementation is removed or if you're consolidating into a single function. * **Impact:** While removing duplicates is generally low impact, address all points. * **Feature:** NO (unless removing one implementation impacts functionality) * **User/Build:** Likely NO, but state explicitly * **Hardware:** Likely NO, but state explicitly * **Documentation:** Maybe YES if function names changed, requiring API doc updates * **Security/Compatibility:** Likely NO, but confirm * **Testing:** * **Build Host:** Specify your OS, compiler, and version for reproducibility. * **Target:** "sim" is vague. Which simulator and architecture? * **Logs:** Provide snippets of relevant logs showing the change's effect (e.g., function call before/after, resource usage). **Example for Improvement (Testing):** ``` ## Testing I confirm that changes are verified on local setup and works as intended: * Build Host(s): Ubuntu 20.04, GCC 9.4.0 * Target(s): qemu-system-arm (Cortex-M3), configs/sim/nsh/defconfig Testing logs before change: ``` [sched] Starting CPU load monitor [sched] nxsched_critmon_cpuload: CPU load = 15% (implementation 1) ``` Testing logs after change: ``` [sched] Starting CPU load monitor [sched] nxsched_critmon_cpuload: CPU load = 15% (consolidated implementation) ``` -- 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]
