Skip to content

feat: identify - #286

Merged
p-shahi merged 11 commits into
refactor/introfrom
protocols/identify
Feb 20, 2023
Merged

feat: identify#286
p-shahi merged 11 commits into
refactor/introfrom
protocols/identify

Conversation

@salmad3

@salmad3salmad3 commented Jan 9, 2023•
edited
Loading

Copy link
Copy Markdown
Contributor

Context

Adds identify content to protocols section.

Latest preview

Fleek preview unavailable as PR not against master.

@salmad3salmad3 added the ready for review PR is ready for review label Jan 9, 2023
marten-seemann
marten-seemann reviewed Jan 11, 2023
View reviewed changes

@marten-seemannmarten-seemann left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should also mention that there's a Identify Delta that go-libp2p shipped with for some time, and that we recently deprecated because we found that it's not particularly useful, but introduced a lot of complexity.

Comment threadcontent/concepts/introduction/protocols/identify.md Outdated
Comment threadcontent/concepts/introduction/protocols/identify.md Outdated
@salmad3salmad3 added the P1 High label Jan 26, 2023
marten-seemann
marten-seemann reviewed Jan 31, 2023
View reviewed changes
Comment threadcontent/concepts/introduction/protocols/identify.md Outdated
Comment threadcontent/concepts/introduction/protocols/identify.md Outdated
Comment threadcontent/concepts/introduction/protocols/identify.md Outdated
Comment threadcontent/concepts/introduction/protocols/identify.md Outdated
Comment threadcontent/concepts/introduction/protocols/identify.md Outdated
remote node and the remote node responds with an `Identify` message containing
its metadata and closes the stream.

<!-- ADD Diagram -->

@marten-seemannmarten-seemannJan 31, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not sure if we need one, this should be really straightforward.

@salmad3salmad3Feb 1, 2023

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I had sketched simple diagrams, one for identity and identify/push.

Comment threadcontent/concepts/introduction/protocols/identify.md Outdated
Comment threadcontent/concepts/introduction/protocols/identify.md Outdated
Comment threadcontent/concepts/introduction/protocols/identify.md Outdated
@salmad3salmad3 added change request PR requires changes. and removed ready for review PR is ready for review labels Jan 31, 2023
salmad3and others added 2 commits February 1, 2023 08:34
@salmad3@marten-seemann
Apply suggestions from code review
746de91
Co-authored-by: Marten Seemann <martenseemann@gmail.com>
@salmad3
edits
cdc2aee
@salmad3salmad3 added ready for review PR is ready for review and removed change request PR requires changes. labels Feb 1, 2023
mxinden
mxinden approved these changes Feb 20, 2023
View reviewed changes

@mxindenmxinden left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Pushed a couple of adjustments. This is good to go from my end.

@p-shahi
p-shahi merged commit 9f5367a into refactor/introFeb 20, 2023
@p-shahi
p-shahi deleted the protocols/identify branch February 20, 2023 16:11
p-shahi pushed a commit that referenced this pull request Mar 1, 2023
@salmad3@p-shahi
feat: identify (#286)
94572fc
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mxindenmxindenmxinden approved these changes
@p-shahip-shahip-shahi approved these changes
@MarcoPoloMarcoPoloAwaiting requested review from MarcoPolo
@marten-seemannmarten-seemannAwaiting requested review from marten-seemann

Assignees

No one assigned

Labels

P1Highready for reviewPR is ready for review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@salmad3@marten-seemann@mxinden@p-shahi