-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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(cli): add daemon option --agent-version-suffix #8419
Conversation
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 feel we want to keep as much as possible inside version.go
– see comment inline (#8419 (comment))
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.
Move to version.go
looks ok.
CI does not pass (sharness needs a fix - https://app.circleci.com/pipelines/github/ipfs/go-ipfs/5325/workflows/0fef1461-682c-4f58-8f31-c66cfab0a694/jobs/58264), need to fix before merge.
4523363
to
f5ba05a
Compare
@lidel Sharness passing now (didn't really do anything besides squashing and triggering another round of testing; this was already passing locally on my machine). |
(The tests themselves are passing, the error seems unrelated to this PR and more internal with CI dynamics but let me know if you'd like another run or any modification on this patch.) |
@schomatis Rebase this PR to pick up CI changes that will likely allow your sharness tests to pass. |
f5ba05a
to
a54d54b
Compare
Rebased. Still hitting same error. |
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.
@schomatis seems that diff -u expected-agent-version actual-agent-version
fails on CI because of duplicate slash:
--- expected-agent-version 2021-09-13 12:30:02.643061595 +0000
+++ actual-agent-version 2021-09-13 12:30:02.703065735 +0000
@@ -1 +1 @@
-go-ipfs/0.11.0-dev//test-suffix
+go-ipfs/0.11.0-dev/test-suffix
It pass locally for me, but on CI we seem to get an empty AGENT_COMMIT
– does this ring any bells?
@lidel Thanks for pasting the diff here; I couldn't extract it from the Circle UI. Fixed the sharness test to cover both cases of the commit being present or not. |
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.
This seems reasonable to me. LGTM.
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.
Merging to unlock use in Brave when 0.10 ships.
* feat(cli): add daemon option --agent-version-suffix * fix sharness test when commit is empty (release) (cherry picked from commit 3a84352)
* feat(cli): add daemon option --agent-version-suffix * fix sharness test when commit is empty (release) (cherry picked from commit 3a84352)
* feat(cli): add daemon option --agent-version-suffix * fix sharness test when commit is empty (release) (cherry picked from commit 3a84352)
Preliminary design for feedback to add the option storing it inside the
IpfsNode
structure.(Test and minor fixes will follow after this design is validated.)
Closes #7898.