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.

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

Merged
merged 2 commits into from
Sep 21, 2021

Conversation

schomatis
Copy link
Contributor

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.

Copy link
Member

@lidel lidel left a 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))

cmd/ipfs/daemon.go Outdated Show resolved Hide resolved
cmd/ipfs/daemon.go Outdated Show resolved Hide resolved
version.go Outdated Show resolved Hide resolved
core/core.go Outdated Show resolved Hide resolved
Copy link
Member

@lidel lidel left a 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.

@Stebalien Stebalien removed their request for review September 9, 2021 14:51
@schomatis schomatis force-pushed the schomatis/cli/agent-version-suffix branch from 4523363 to f5ba05a Compare September 10, 2021 12:40
@schomatis schomatis marked this pull request as ready for review September 10, 2021 12:43
@schomatis
Copy link
Contributor Author

@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).

@schomatis
Copy link
Contributor Author

(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.)

@gammazero
Copy link
Contributor

@schomatis Rebase this PR to pick up CI changes that will likely allow your sharness tests to pass.

@schomatis schomatis force-pushed the schomatis/cli/agent-version-suffix branch from f5ba05a to a54d54b Compare September 13, 2021 12:09
@schomatis
Copy link
Contributor Author

Rebased. Still hitting same error.

Copy link
Member

@lidel lidel left a 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?

@schomatis
Copy link
Contributor Author

@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.

Copy link
Contributor

@petar petar left a 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.

@BigLep BigLep added this to the go-ipfs 0.11 milestone Sep 21, 2021
Copy link
Member

@lidel lidel left a 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.

@lidel lidel merged commit 3a84352 into master Sep 21, 2021
@lidel lidel deleted the schomatis/cli/agent-version-suffix branch September 21, 2021 18:31
@BigLep BigLep removed this from the go-ipfs 0.11 milestone Sep 23, 2021
aschmahmann pushed a commit that referenced this pull request Sep 27, 2021
* feat(cli): add daemon option --agent-version-suffix
* fix sharness test when commit is empty (release)

(cherry picked from commit 3a84352)
aschmahmann pushed a commit that referenced this pull request Sep 27, 2021
* feat(cli): add daemon option --agent-version-suffix
* fix sharness test when commit is empty (release)

(cherry picked from commit 3a84352)
aschmahmann pushed a commit that referenced this pull request Sep 27, 2021
* feat(cli): add daemon option --agent-version-suffix
* fix sharness test when commit is empty (release)

(cherry picked from commit 3a84352)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Optional ipfs daemon --agent-version-suffix=
5 participants