Skip to content
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

README.md: update supported flags table #791

Merged
merged 1 commit into from
Jan 14, 2025
Merged

Conversation

mvo5
Copy link
Collaborator

@mvo5 mvo5 commented Jan 10, 2025

This commit updates the documentation around the supported flags. It also switches on "bold" for a smaller subset of the important flags.

Note that still not all flags are shows, some are documented below (like the --aws-*) and some (like --store, --rpmmd) require more context, i.e. explaining that they require calling podman with the right --volume mappings.

@mvo5 mvo5 requested review from ondrejbudai and supakeen January 10, 2025 14:16
Copy link

@p5 p5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hope you don't mind a drive-by review :)

I've spent quite a lot of time in these docs over the past week, so thought I could be helpful.

| **--target-arch** | [Target arch](#-target-architecture) to build | ❌ |
| **--verbose** | Switch output/progress to verbose mode | `false` |
| --tls-verify | Require HTTPS and verify certificates when contacting registries | `true` |
| **--type** | [Image type](#-image-types) to build (can be passed multiple times) | `qcow2` |
Copy link

@p5 p5 Jan 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a mix of information on how --type should be passed in. One place says it can be passed in multiple times (--type qcow2 --type raw), and another says to use a string array (--type [qcow2,raw]).

Even if both are supported, I think it would be good to standardise on which is recommended in the docs to avoid confusion. Or mentioning both in the same sentence/section, saying "both are supported".

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the README to clarify this a bit more, part of the issue is the language from cobra, it auto-generates the "stringArray" word. I would have to look into cobra to see how to workaround/replace this if it's too confusing.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's start here and turn the above comment into an issue (I'll click the button).

README.md Outdated
--target-arch string build for the given target architecture (experimental)
--tls-verify require HTTPS and verify certificates when contacting registries (default true)
--type stringArray image types to build [ami, anaconda-iso, gce, iso, qcow2, raw, vhd, vmdk] (default [qcow2])
--version version for build
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
--version version for build
--version version for bootc-image-builder

"version for build" is slightly confusing because I'm not sure whether it's the version of my image build or the BIB image.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, thank you! I fixed this now in the README. Fixing this in the code is a bit more work as we need to workaround cobra here.

Copy link
Member

@ondrejbudai ondrejbudai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I agree with @p5's concerns. :)

This commit updates the documentation around the supported flags.
It also switches on "bold" for a smaller subset of the important
flags.

Note that still not all flags are shows, some are documented
below (like the `--aws-*`) and some (like --store, --rpmmd)
require more context, i.e. explaining that they require calling
podman with the right `--volume` mappings.
Copy link
Member

@ondrejbudai ondrejbudai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@supakeen supakeen enabled auto-merge January 13, 2025 09:24
@mvo5 mvo5 disabled auto-merge January 14, 2025 08:31
@mvo5 mvo5 merged commit fd0f7c3 into osbuild:main Jan 14, 2025
7 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants