-
Notifications
You must be signed in to change notification settings - Fork 18
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
feat: add build config configurator #1026
base: main
Are you sure you want to change the base?
Conversation
56ffd07
to
c8f64ec
Compare
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.
Very nice improvement 👍🏼, worked well in testing but just had some usability suggestions.
packages/frontend/src/Build.svelte
Outdated
Supplying the following fields will create a build config file that contains the build options for the | ||
disk image. Customizations include user, password, SSH keys and kickstart files. More information can | ||
be found in the <Link | ||
externalRef="https://github.com/osbuild/bootc-image-builder?tab=readme-ov-file#-build-config" |
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.
Need to escape the # sign? It should open:
https://github.com/osbuild/bootc-image-builder?tab=readme-ov-file#-build-config
but it is currently opening:
https://github.com/osbuild/bootc-image-builder?tab=readme-ov-file##-build-config
(goes to the top instead of the correct section).
packages/frontend/src/Build.svelte
Outdated
{#if showBuildConfig} | ||
<p class="text-sm text-[var(--pd-content-text)] mb-2"> | ||
Supplying the following fields will create a build config file that contains the build options for the | ||
disk image. Customizations include user, password, SSH keys and kickstart files. More information can |
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.
The middle sentence ('Customizations ... files') is sort of redundant when you look at the options, I would just remove it to simplify.
packages/frontend/src/Build.svelte
Outdated
<Input | ||
bind:value={filesystem.minsize} | ||
id="buildConfigFilesystemMinimumSize.${index}" | ||
placeholder="Minimum size (ex. 30 GiB)" /> |
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.
It wasn't clear to me if the units had to be specified or not, then I used Gb without thinking and it failed, so I looked in the docs and couldn't find what's supported either. Maybe if we put the '30 GiB' in quotes it'll be more obvious that's a direct example?
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.
Yeah, I had that same issue too :( in the build config it's explicitly GiB, i'll put it in quotes!
packages/frontend/src/Build.svelte
Outdated
externalRef="https://github.com/osbuild/bootc-image-builder?tab=readme-ov-file#-build-config" | ||
>bootc-image-builder documentation</Link | ||
>. | ||
This will override any above user-specific input and use the supplied file only. |
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.
The text at the top only talks about filling in the fields, then at the bottom of the form you find out you can also provide a file. Feels like maybe there should be two radio buttons like 'Create new build config' and 'Supply existing build config file' so that it's clear you have either option from the start?
{#if showAdvanced} | ||
{#if showBuildConfig} | ||
<p class="text-sm text-[var(--pd-content-text)] mb-2"> | ||
Supplying the following fields will create a build config file that contains the build options for the |
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.
It never says where the build config will be generated. I think this would help (e.g. say we put it in the folder above), so that I could use this once to create it, then maybe edit it outside of Podman Desktop, then supply the same file next time I build.
### What does this PR do? * Adds automated way to create a build config json file * Added to build page ### Screenshot / video of UI <!-- If this PR is changing UI, please include screenshots or screencasts showing the difference --> ### What issues does this PR fix or reference? <!-- Include any related issues from Podman Desktop repository (or from another issue tracker). --> Closes podman-desktop#827 ### How to test this PR? <!-- Please explain steps to reproduce --> Use build config and configure / create your own custom build config with users, filesystem, etc. Signed-off-by: Charlie Drage <[email protected]>
adcfdea
to
9c6d2a2
Compare
I've updated the PR with the following:
|
Signed-off-by: Charlie Drage <[email protected]>
9c6d2a2
to
f86033d
Compare
feat: add build config configurator
What does this PR do?
Screenshot / video of UI
Screen.Recording.2024-11-18.at.3.44.07.PM.mov
What issues does this PR fix or reference?
Closes #827
How to test this PR?
Use build config and configure / create your own custom build config
with users, filesystem, etc.