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.

Sign up for GitHub

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

Jump to bottom

bootc-image-builder/main: extend version command (HMS-5211) #761

Merged
merged 3 commits into from
Jan 7, 2025
Merged

bootc-image-builder/main: extend version command (HMS-5211) #761

merged 3 commits into from
Jan 7, 2025

Conversation

schuellerf
Copy link
Contributor

@schuellerf schuellerf commented Dec 17, 2024 •
edited
Loading

Extend version with printing also the timestamp and "tainted" if not all files are checked in.
Also support calling it with --version, -v or version.

JIRA: HMS-5211

@schuellerf schuellerf force-pushed the enable-version-command branch from a335203 to 511e009 Compare December 17, 2024 14:07
@schuellerf
Copy link
Contributor Author

/jira-epic HMS-5208

@schutzbot schutzbot changed the title bootc-image-builder/main: extend version command bootc-image-builder/main: extend version command (HMS-5211) Dec 17, 2024
@schuellerf schuellerf force-pushed the enable-version-command branch from 511e009 to 47b150a Compare December 18, 2024 08:30
mvo5
mvo5 reviewed Dec 19, 2024
View reviewed changes
Copy link
Collaborator

@mvo5 mvo5 left a comment

Choose a reason for hiding this comment

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

Thank you! I added some inline ideas/thoughts for your consideration :)

bib/cmd/bootc-image-builder/main.go Outdated Show resolved Hide resolved
bib/cmd/bootc-image-builder/main.go Outdated Show resolved Hide resolved
bib/cmd/bootc-image-builder/main.go Outdated Show resolved Hide resolved
bib/cmd/bootc-image-builder/main.go Show resolved Hide resolved
@schuellerf schuellerf force-pushed the enable-version-command branch from 47b150a to 889109f Compare December 20, 2024 18:33
@schuellerf schuellerf requested a review from mvo5 December 20, 2024 18:34
@schuellerf schuellerf force-pushed the enable-version-command branch from 889109f to 55aee70 Compare December 20, 2024 18:35
ondrejbudai
ondrejbudai reviewed Jan 6, 2025
View reviewed changes
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.

Idea: the format is almost yaml. What if we made a few adjustments, and actually make it yaml? This way, it would be parseable both by human eyes, and by yaml parsers (e.g. yq).

bootc does this, and I kinda like it. Ftr, I don't we need to use a fully fledged yaml library I think, our output is super simple.

@schuellerf
Copy link
Contributor Author

@ondrejbudai this actually is yaml compatible - even yamllint is ok with it (when adding document-start)
But I can for sure replace the spaces in the "keys" with underscores, this looks nicer.
Any other changes you would suggest? Quoting the values is neither needed nor easier to read/parse, IMHO.

@schuellerf
bootc-image-builder/main: extend version command
54ae0d7 
Extend version with printing also the timestamp and
"tainted" if not all files are checked in.
Also support calling it with `--version` or `-v`.
@schuellerf
cmd/bootc-image-builder/main: Change version output parser friendly
0e75c2c 
Changes the version output to be more parser friendly
for simple split or yaml parsers.
@schuellerf
cmd/bootc-image-builder/main: Introduce error handling
8cbd0d0 
Error handling on argument parsing. Remark from golangci-lint.
@schuellerf schuellerf force-pushed the enable-version-command branch from 55aee70 to 8cbd0d0 Compare January 7, 2025 11:10
@schuellerf
Copy link
Contributor Author

The space after build originates from cobra but can be avoided, please check if this is better now:

build_revision: 8cbd0d0
build_time: 2025-01-07T11:09:18Z
build_status: ok

@schuellerf schuellerf requested a review from ondrejbudai January 7, 2025 11:12
ondrejbudai
ondrejbudai approved these changes Jan 7, 2025
View reviewed changes
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.

Awesome! :) I didn't know that yaml can have spaces in keys. Thanks!

@ondrejbudai ondrejbudai enabled auto-merge January 7, 2025 11:34
@ondrejbudai ondrejbudai added this pull request to the merge queue Jan 7, 2025
Merged via the queue into osbuild:main with commit 2ec8b7e Jan 7, 2025
11 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ondrejbudai ondrejbudai ondrejbudai approved these changes

@mvo5 mvo5 Awaiting requested review from mvo5

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@schuellerf @ondrejbudai @mvo5