Follow-up to #319#329
Conversation
"main: Change some logs from Info to Debug" moved some logs to debug, but an unneeded blank line was kept. This commit removes it. Assisted-by: CodeRabbit Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
A few places in the documentation had " " instead of just " ". This commit removes them. Assisted-by: CodeRabbit Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Assisted-by: CodeRabbit Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Reviewer's GuideThis PR addresses minor stylistic and formatting comments from #319 by refining documentation spacing, improving script quoting, and removing an unnecessary log statement. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
WalkthroughThis update removes an informational log statement from the VM configuration code, adjusts shell script argument quoting for robustness, and makes minor formatting corrections in the documentation. No changes were made to logic, exported entities, or overall functionality. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
![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 (3)
doc/usage.md (1)
14-20: Consider dropping the hyphen in “log-level”.Inside normal prose “log level” is more idiomatic English; the hyphen is only required in the flag itself (
--log-level).
Example tweak:-Set the log-level for VFKit. +Set the log level for VFKit.Purely cosmetic, feel free to ignore if you prefer the current wording.
contrib/scripts/start-gvproxy.sh (2)
29-31: Quote variables that may contain spaces.
cpand thekill/catsequence will break on disk-image names or directories containing spaces.-trap 'if [[ -f "${VM_NAME}.gvproxy.pid" ]]; then kill $(cat ${VM_NAME}.gvproxy.pid); fi; rm -f ${TO_REMOVE}' EXIT -cp -c ${DISK_IMAGE} "${VM_NAME}".overlay.img +trap 'if [[ -f "${VM_NAME}.gvproxy.pid" ]]; then kill "$(cat "${VM_NAME}.gvproxy.pid")"; fi; rm -f ${TO_REMOVE}' EXIT +cp -c "${DISK_IMAGE}" "${VM_NAME}.overlay.img"
33-38: Guard against spaces in the socket path.
$(pwd)might legally contain spaces, which would split the argument after the comma.
A minimal change keeps the value intact:- --device virtio-net,unixSocketPath=$(pwd)/${VM_NAME}.sock,mac=5a:94:ef:e4:0c:ee \ + --device virtio-net,unixSocketPath="$(pwd)/${VM_NAME}.sock",mac=5a:94:ef:e4:0c:ee \Shell-parsing remains unchanged for vfkit.
Optional but improves robustness.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
cmd/vfkit/main.go(0 hunks)contrib/scripts/start-gvproxy.sh(1 hunks)doc/usage.md(2 hunks)
💤 Files with no reviewable changes (1)
- cmd/vfkit/main.go
🧰 Additional context used
🪛 LanguageTool
doc/usage.md
[grammar] ~14-~14: Don’t hyphenate words unnecessarily.
Context: ...ic Options - --log-level Set the log-level for VFKit. Supported values are `d...
(QB_NEW_EN_OTHER_ERROR_IDS_000042)
[grammar] ~14-~14: Use proper spacing conventions.
Context: ...values are debug, info, and error. - --restful-uri The URI (address) of the RESTful service...
(QB_NEW_EN_OTHER_ERROR_IDS_000007)
[grammar] ~18-~18: There might be a mistake here.
Context: ...default it’s disabled. Valid schemes are tcp, none, or unix. In the case of...
(QB_NEW_EN_OTHER)
[grammar] ~19-~19: Use proper spacing conventions.
Context: ... of none disables the RESTful service. ### Virtual Machine Resources These options...
(QB_NEW_EN_OTHER_ERROR_IDS_000007)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: lint
- GitHub Check: build (macOS-14)
- GitHub Check: build (macOS-13)
🔇 Additional comments (1)
contrib/scripts/start-gvproxy.sh (1)
26-26: Quoting fix is spot-on.Wrapping
${GVPROXY}and the--listen-vfkitURL in quotes eliminates failures when paths contain spaces.
Nothing else to add here.
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: lstocchi The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
2 participants
There were several minor comments from coderabbit in #319
which were valuable. This commit addresses these.
Summary by Sourcery
Apply minor style and cleanup changes across documentation, scripts, and code
Enhancements:
Documentation:
Summary by CodeRabbit
Bug Fixes
Documentation
Chores