Fix: PHP error on new install#2385
Conversation
🔧 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 review |
✅ Actions performedReview triggered.
|
WalkthroughSingle-file edit updates header year and initializes a temporary string variable before concatenation when writing a cfg file. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
emhttp/plugins/dynamix.vm.manager/scripts/libvirtconfig (4)
38-39: Optional: avoid .= in loop; build once and implode.Reduces repeated string reallocations and is a bit clearer.
- foreach ($cfg_new as $key => $value) $tmp .= "$key=\"$value\"\n"; + $lines = []; + foreach ($cfg_new as $key => $value) { $lines[] = "$key=\"$value\""; } + $tmp = implode("\n", $lines) . "\n";
39-39: Add file lock for safer writes.Prevents concurrent processes from clobbering the cfg during writes.
- file_put_contents($cfgfile, $tmp); + file_put_contents($cfgfile, $tmp, LOCK_EX);
29-34: Use key-aware comparison; array_diff ignores keys.array_diff() compares values only; duplicate values across different keys can yield a false “no changes” result. Compare assoc diffs both ways or normalize and compare.
- if (empty(array_diff($cfg_new, $cfg_old))) unset($cfg_new); + if (empty(array_diff_assoc($cfg_new, $cfg_old)) && empty(array_diff_assoc($cfg_old, $cfg_new))) { + unset($cfg_new); + }
14-14: Avoid short open tag.Replace
<?with<?phpto remove short_open_tag dependency in varied PHP configs.-<? +<?php
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
emhttp/plugins/dynamix.vm.manager/scripts/libvirtconfig(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: Squidly271
PR: unraid/webgui#0
File: :0-0
Timestamp: 2025-05-31T05:10:13.120Z
Learning: Squidly271 prefers not to receive CodeRabbit reviews or feedback while PRs are in draft status. Only provide reviews after PRs are moved out of draft.
🔇 Additional comments (2)
emhttp/plugins/dynamix.vm.manager/scripts/libvirtconfig (2)
3-3: Header year update is correct.Matches current year and is consistent with the file history.
37-37: Good fix: initialize $tmp before concatenation.Prevents undefined-variable notices on first-run installs when building the cfg content.
4 participants
Summary by CodeRabbit
Bug Fixes
Chores