Skip to content

[RFC] main: print pretty json manifest#414

Merged
lzap merged 1 commit intoosbuild:mainfrom
mvo5:pretty-manifests
Dec 18, 2025
Merged

[RFC] main: print pretty json manifest#414
lzap merged 1 commit intoosbuild:mainfrom
mvo5:pretty-manifests

Conversation

@mvo5
Copy link
Contributor

@mvo5 mvo5 commented Dec 17, 2025

This commit makes the json pretty when image-builder manifest is used. Given that this is most likley something that our users run to actually see the manifest we should make it pretty by default.

This commit makes us json pretty the osbuild manifests.
This means that when `image-builder manifest` or
`image-builder build --with-manifest` is used we get a
nice representation of the manifest (and it has no
consequences otherwise except for a few more bytes in
memory for the extra whitespace).

Given that people who ask for a manifest often want to
inspect it seems nice to make it pretty by default. OTOH
there is jq - so if its considered fluff that is fine too :)
@mvo5 mvo5 requested a review from a team as a code owner December 17, 2025 09:27
@mvo5 mvo5 requested review from bcl, croissanne and supakeen and removed request for a team December 17, 2025 09:27
@lzap
Copy link
Contributor

lzap commented Dec 17, 2025

Love it but here is an idea. Interactive terminal? Pretty print. In other cases (pipe or redirect) do ugly output. We already have a helper for that for the progress bar, you could re-use it.

Copy link
Member

@supakeen supakeen left a comment

Choose a reason for hiding this comment

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

Thanks, saves me a pipe.

Copy link
Contributor

@lzap lzap left a comment

Choose a reason for hiding this comment

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

I mean this is better than before sure, no need to make it too complex. I would go for two spaces myself but you are the inventor here :-)

@mvo5
Copy link
Contributor Author

mvo5 commented Dec 17, 2025

I mean this is better than before sure, no need to make it too complex. I would go for two spaces myself but you are the inventor here :-)

Haha - I absolutely do not mind, I just copied the json.Ident() from the upstream docs, if you prefer 2 instead of 4 I will gladly change it, just say the word!

@lzap lzap added this pull request to the merge queue Dec 18, 2025
@lzap
Copy link
Contributor

lzap commented Dec 18, 2025

Haha - I absolutely do not mind, I just copied the json.Ident() from the upstream docs, if you prefer 2 instead of 4 I will gladly change it, just say the word!

As you already have figured out, I am very, very rarely making someone else change their code. I only offer other view and if they do not fall in love in these (often crazy) ideas, that is fine :-)

Let's do it. Thanks.

Merged via the queue into osbuild:main with commit e205501 Dec 18, 2025
35 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.

3 participants