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

Get hirte version through CLI #404

Merged
merged 1 commit into from
Jul 31, 2023
Merged

Get hirte version through CLI #404

merged 1 commit into from
Jul 31, 2023

Conversation

ArtiomDivak
Copy link
Contributor

@ArtiomDivak ArtiomDivak commented Jul 31, 2023 •
edited by mwperina
Loading

Added CLI parameter for hirte, hirte-agent, hirtectl and hirte-proxy that prints thier current version.

Fixes: #384

engelmi
engelmi reviewed Jul 31, 2023
View reviewed changes
Copy link
Member

@engelmi engelmi left a comment

Choose a reason for hiding this comment

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

A few nits, otherwise looks great! :)

meson.build Outdated Show resolved Hide resolved
src/agent/main.c Outdated Show resolved Hide resolved
src/manager/main.c Outdated Show resolved Hide resolved
src/proxy/main.c Outdated Show resolved Hide resolved
mwperina
mwperina requested changes Jul 31, 2023
View reviewed changes
meson.build Outdated Show resolved Hide resolved
src/proxy/main.c Outdated Show resolved Hide resolved
@mwperina
Copy link
Member

I would also be great to start referencing relevant GH issue within commit message and PR description, so the issues is automatically closed when PR is merged:

Get hirte version through CLI

Added CLI parameter for hirte, hirte-agent, hirtectl and hirte-proxy
that prints thier current version.

Fixes: https://github.com/containers/hirte/issues/384
Signed-off-by: Artiom Divak <[email protected]>

mwperina
mwperina reviewed Jul 31, 2023
View reviewed changes
Copy link
Member

@mwperina mwperina left a comment

Choose a reason for hiding this comment

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

One naming suggestion, other than that it looks great

meson.build Outdated Show resolved Hide resolved
@ArtiomDivak ArtiomDivak force-pushed the get-ver branch 2 times, most recently from fabdf0e to feb688c Compare July 31, 2023 12:01
@engelmi
Copy link
Member

engelmi commented Jul 31, 2023

Just came to mind, could you please also update the man pages:
https://github.com/containers/hirte/tree/main/doc/man

@ArtiomDivak
Get hirte version through CLI
250d82c 
Added CLI parameter for hirte, hirte-agent, hirtectl and hirte-proxy
that prints thier current version.

Fixes: eclipse-bluechi#384
Signed-off-by: Artiom Divak <[email protected]>
engelmi
engelmi approved these changes Jul 31, 2023
View reviewed changes
Copy link
Member

@engelmi engelmi left a comment

Choose a reason for hiding this comment

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

LGTM
Great work!

mwperina
mwperina approved these changes Jul 31, 2023
View reviewed changes
Copy link
Member

@mwperina mwperina left a comment

Choose a reason for hiding this comment

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

LGTM

@engelmi engelmi merged commit 421adc1 into eclipse-bluechi:main Jul 31, 2023
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@engelmi engelmi engelmi approved these changes

@mwperina mwperina mwperina approved these changes

@alexlarsson alexlarsson Awaiting requested review from alexlarsson alexlarsson is a code owner

@mkemel mkemel Awaiting requested review from mkemel mkemel is a code owner

@rhatdan rhatdan Awaiting requested review from rhatdan rhatdan is a code owner

@sdunnagan sdunnagan Awaiting requested review from sdunnagan

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

Successfully merging this pull request may close these issues.

Add CLI option to get the version of the binaries
3 participants
@ArtiomDivak @mwperina @engelmi