You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "Gary-Hobson (via GitHub)" <gi...@apache.org> on 2024/10/15 04:42:01 UTC

[PR] sched: remove duplicate functions [nuttx]

Gary-Hobson opened a new pull request, #14286:
URL: https://github.com/apache/nuttx/pull/14286

   ## Summary
   sched: remove duplicate functions
   nxsched_critmon_cpuload has two identical implementations
   
   ## Impact
   
   ## Testing
   sim


-- 
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-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] sched: remove duplicate functions [nuttx]

Posted by "GUIDINGLI (via GitHub)" <gi...@apache.org>.
GUIDINGLI merged PR #14286:
URL: https://github.com/apache/nuttx/pull/14286


-- 
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-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] sched: remove duplicate functions [nuttx]

Posted by "nuttxpr (via GitHub)" <gi...@apache.org>.
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: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org