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

Version handshake #180

Merged
merged 9 commits into from
Oct 16, 2014
Merged

Version handshake #180

merged 9 commits into from
Oct 16, 2014

Conversation

cryptix
Copy link
Contributor

@cryptix cryptix commented Oct 16, 2014

addresses #166

optional int64 minor = 2;
optional int64 patch = 3;
// BUG(cryptix): do we need PreRelease and Metadata too?
}
Copy link
Member

Choose a reason for hiding this comment

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

Let's do this instead:

message Handshake1 {
    // protocolVersion determines compatibility between peers
    optional string protocolVersion = 1; // semver

    // agentVersion is like a UserAgent string in browsers, or client version in bittorrent
    // includes the client name and client. e.g.   "go-ipfs/0.1.0"
    optional string agentVersion = 2; // semver 

    // we'll have more fields here later.
}

We dont need protobof to parse semver, it's a well established format, and we can parse it separately. (go-semver)

@jbenet
Copy link
Member

jbenet commented Oct 16, 2014

Hey @cryptix, CR done. Major points:

  • net/version -> net/handshake
  • use go-semver and version strings
  • simplify the select business (happy to do this, too, i've a pretty good sense of what i meant and may not have communicated it perfectly)

@jbenet
Copy link
Member

jbenet commented Oct 16, 2014

@cryptix this is great! thanks!

@jbenet
Copy link
Member

jbenet commented Oct 16, 2014

Ok, all set! Merged. Thanks!

jbenet added a commit that referenced this pull request Oct 16, 2014
@jbenet jbenet merged commit bbfba91 into ipfs:master Oct 16, 2014
This was referenced Oct 16, 2014
@cryptix cryptix deleted the versionHandshake branch October 16, 2014 12:42
@aschmahmann aschmahmann mentioned this pull request May 14, 2021
71 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants