-
Notifications
You must be signed in to change notification settings - Fork 103
Add tag
& commit_hash
field to GetInfo
response
#1034
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
base: master
Are you sure you want to change the base?
Add tag
& commit_hash
field to GetInfo
response
#1034
Conversation
commit_hash
field to GetInfo
response
litrpc/proxy.proto
Outdated
@@ -54,4 +54,7 @@ message GetInfoRequest { | |||
message GetInfoResponse { | |||
// The version of the LiTd software that the node is running. | |||
string version = 1; | |||
|
|||
// The git commit hash of the litd binary. | |||
string commit_hash = 2; |
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.
Let me know if you think commit
would be a more suitable name for this, as the contents may be the tag
rather than a commit hash. But opted for this naming as it then mimics the loop
getinfo response :)
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.
Staying consistent is probably better, I would have chosen "revision" I guess.
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.
LGTM, nice ⚡
edit: will still check the build
edit2: reproduced the build 🎉
litrpc/proxy.proto
Outdated
@@ -54,4 +54,7 @@ message GetInfoRequest { | |||
message GetInfoResponse { | |||
// The version of the LiTd software that the node is running. | |||
string version = 1; | |||
|
|||
// The git commit hash of the litd binary. | |||
string commit_hash = 2; |
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.
Staying consistent is probably better, I would have chosen "revision" I guess.
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.
I guess I dont love the "it's sometimes this but then sometimes that" part.
Cant we just leave "Commit" as it is (to not change behaviour there) but then add "Commit_hash" which is always the revision hash no matter what?
@ViktorTigerstrom, remember to re-request review from reviewers when ready |
e0b7fc5
to
90882c6
Compare
Add the `tag` and `commit_hash` fields to the GetInfoResponse. The semantics of the `version` field is also updated to always contain the most recent semantic version of the litd node, following the semantic versioning 2.0.0 spec (http://semver.org/). This commit also updates the `litcli --version` command result, to include all three fields of the GetInfoResponse.
90882c6
to
49af0b0
Compare
Thanks for the reviews! I've updated the PR to address the reviews, by instead adding a The The contents of the @bitromortac & @ellemouton, can you please check the updates, and check if you think the outputs of the |
commit_hash
field to GetInfo
responsetag
& commit_hash
field to GetInfo
response
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.
LGTM, tACK 🎉
Mimics the behaviour of loop pull 914 (lightninglabs/loop#914), but for
litd
.This change adds a new field
commit_hash
field to theGetInfo
response, which will include thelitd
tag
if the currently runninglitd
daemon is built on a a clean tag. If thelitd
version is not built on a clean tagged version, this field will instead include the commit hash that the build is based on.The contents of
version
field of theGetInfo
response is also will also be changed, so that the it'll only include thelitd
version
, and not the commit_hash/tag.Note to reviewers:
Just to ensure that this doesn't break the reproducible build across different hosts:
Could you please create a local tag on top of this:
git tag -s v123456789.123456789.123456789-alpha
make docker-release tag=v123456789.123456789.123456789-alpha
if you're on mac, ormake release tag=v123456789.123456789.123456789-alpha
with the correct GO version if you're on Linux.sha256
output for the./lightning-terminal-v123456789.123456789.123456789-alpha/manifest-v123456789.123456789.123456789-alpha.txt
file is:e9bc6a43642d7ae679f1f2bd809c3a517cefc0bddb32beee5e990fc60bd75783