fix: parse dmidecode memory capacity labels consistently across versions#2562
fix: parse dmidecode memory capacity labels consistently across versions#2562
Conversation
WalkthroughThe changes normalize memory unit handling across two plugin files by introducing consistent memory units mapping and parsing functions to convert memory size strings to MiB, replacing ad-hoc parsing logic previously used in memory display calculations. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔧 PR Test Plugin AvailableA test plugin has been generated for this PR that includes the modified files. Version: 📥 Installation Instructions:Install via Unraid Web UI:
Alternative: Direct Download
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
emhttp/plugins/dynamix/DashStats.page (2)
193-204: Inconsistent formatting compared tosystem_information.The nested
} else { ifstructure with braces on the same line as content is harder to read than the equivalentelseifchain used insystem_information(lines 139-149). Consider aligning the formatting for consistency:♻️ Suggested formatting improvement
if ($memory_installed >= 1048576) { $memory_installed = round($memory_installed/1048576); $memory_maximum = round($memory_maximum/1048576); $memory_unit = 'TiB'; -} else { -if ($memory_installed >= 1024) { +} elseif ($memory_installed >= 1024) { $memory_installed = round($memory_installed/1024); $memory_maximum = round($memory_maximum/1024); - $memory_unit = 'GiB';} -else $memory_unit = 'MiB'; } + $memory_unit = 'GiB'; +} else { + $memory_unit = 'MiB'; +} $unit = $memory_unit;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@emhttp/plugins/dynamix/DashStats.page` around lines 193 - 204, The memory unit selection block uses a nested "} else { if" style that is inconsistent with the `system_information` formatting; change the logic around `memory_installed`, `memory_maximum`, and `memory_unit` to a clear elseif chain (e.g., if (...) { ... } elseif (...) { ... } else { ... }) and place braces consistently on their own lines to match the `system_information` style, then assign `unit = $memory_unit` as before.
166-180: Consider extracting shared memory parsing logic to avoid duplication.The
$memory_unitsarray and$parse_memory_to_mibfunction are duplicated verbatim fromscripts/system_information. This creates a maintenance burden if the logic needs to be updated in the future.Consider extracting these into a shared helper file (e.g., in
include/Helpers.php) that both files canrequire_once.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@emhttp/plugins/dynamix/DashStats.page` around lines 166 - 180, The $memory_units array and the $parse_memory_to_mib closure are duplicated; extract them into a shared helper (e.g., a common PHP helper that returns the units array and a parseMemoryToMiB function) and replace the local definitions in DashStats.page with a require_once of that helper and calls to the shared function(s) (look for symbols $memory_units and $parse_memory_to_mib to locate the code to remove). Ensure the shared helper exposes a callable parseMemoryToMiB($value) that replicates the current behavior (regex, strtolower, rounding and int cast) and update any other files that currently duplicate the logic to use the same helper.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@emhttp/plugins/dynamix/DashStats.page`:
- Around line 193-204: The memory unit selection block uses a nested "} else {
if" style that is inconsistent with the `system_information` formatting; change
the logic around `memory_installed`, `memory_maximum`, and `memory_unit` to a
clear elseif chain (e.g., if (...) { ... } elseif (...) { ... } else { ... })
and place braces consistently on their own lines to match the
`system_information` style, then assign `unit = $memory_unit` as before.
- Around line 166-180: The $memory_units array and the $parse_memory_to_mib
closure are duplicated; extract them into a shared helper (e.g., a common PHP
helper that returns the units array and a parseMemoryToMiB function) and replace
the local definitions in DashStats.page with a require_once of that helper and
calls to the shared function(s) (look for symbols $memory_units and
$parse_memory_to_mib to locate the code to remove). Ensure the shared helper
exposes a callable parseMemoryToMiB($value) that replicates the current behavior
(regex, strtolower, rounding and int cast) and update any other files that
currently duplicate the logic to use the same helper.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
emhttp/plugins/dynamix/DashStats.pageemhttp/plugins/dynamix/scripts/system_information
2 participants
dmidecode always uses binary math but 3.6 and lower report MB/GB/TB, 3.7 correctly reports MiB/GiB/TiB.
Summary by CodeRabbit