-
Notifications
You must be signed in to change notification settings - Fork 11
docs: Fix various errors #135
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request provides a good set of fixes for the documentation, removing incorrect commands, options, and outdated information. The changes make the documentation more accurate. I've added a few suggestions to further improve the readability and usefulness of the documentation, mainly around link formatting and restoring some helpful, corrected examples for the to-disk command.
| @@ -1 +1,3 @@ | |||
| # HACKING | |||
|
|
|||
| See [../HACKING.md](../HACKING.md) for development instructions. | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For better readability, it's generally preferred to use a clean name for the link text rather than a relative path. How about using HACKING.md as the link text?
| See [../HACKING.md](../HACKING.md) for development instructions. | |
| See [HACKING.md](../HACKING.md) for development instructions. |
| # Contributing | ||
|
|
||
| We welcome contributions to bcvk! Please see [docs/HACKING.md](../HACKING.md) for detailed development instructions. | ||
| We welcome contributions to bcvk! Please see [../HACKING.md](../HACKING.md) for detailed development instructions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For better readability and consistency with other files, it's generally preferred to use a clean name for the link text rather than a relative path. How about using HACKING.md as the link text?
| We welcome contributions to bcvk! Please see [../HACKING.md](../HACKING.md) for detailed development instructions. | |
| We welcome contributions to bcvk! Please see [HACKING.md](../HACKING.md) for detailed development instructions. |
| The `to-disk` command creates disk images from bootc containers. | ||
| It is a wrapper for [bootc install to-disk](https://bootc-dev.github.io/bootc/man/bootc-install-to-disk.8.html). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While it's great that you're removing incorrect information, this change removes all examples, making the documentation less helpful. The bcvk to-disk command has its own options and workflow that are worth documenting with a few examples.
To improve usability, please consider restoring a few corrected examples to give users a quick and accurate starting point. For example:
# Disk Images
The `to-disk` command creates disk images from bootc containers.
It is a wrapper for [bootc install to-disk](https://bootc-dev.github.io/bootc/man/bootc-install-to-disk.8.html), but provides its own options for disk creation and runs the installation inside an ephemeral VM.
## Examples
### Basic Usage
To create a default raw disk image:
```bash
bcvk to-disk quay.io/fedora/fedora-bootc:42 output.imgSpecifying Format and Size
To create a 20GB qcow2 image:
bcvk to-disk --format qcow2 --disk-size 20G quay.io/fedora/fedora-bootc:42 output.qcow2Custom Filesystem
To use the xfs filesystem:
bcvk to-disk --filesystem xfs quay.io/fedora/fedora-bootc:42 output.img
jmarrero
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Remove references to non-existent commands and incorrect options: - Remove non-existent `bcvk libvirt create` command from README - Remove non-existent `bcvk libvirt restart` command - Remove incorrect `--autostart` flag from libvirt-run examples - Remove unsupported disk formats (VHD, VMDK) from disk-images.md and really trim down to be clear it just wraps bootc install. - Remove non-existent `--partition` option - Fix `--size` to correct `--disk-size` flag throughout Fix incorrect references: - Fix binary name from `bck` to `bcvk` in docs/HACKING.md and Justfile - Fix repository URL from cgwalters/bcvk to bootc-dev/bcvk Fix incomplete content: - Complete incomplete sentence in README about ephemeral VMs - Populate empty docs/src/HACKING.md with redirect to ../HACKING.md - Fix path in docs/src/contributing.md Assisted-by: Claude Code (Sonnet 4.5) Signed-off-by: Colin Walters <walters@verbum.org>
1391230 to
c9244df
Compare
Remove references to non-existent commands and incorrect options:
bcvk libvirt createcommand from READMEbcvk libvirt restartcommand--autostartflag from libvirt-run examples--partitionoption--sizeto correct--disk-sizeflag throughoutFix incorrect references:
bcktobcvkin docs/HACKING.md and JustfileFix incomplete content:
Assisted-by: Claude Code (Sonnet 4.5)