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

docs: fix some comments errors #2391

Merged
merged 1 commit into from
Jun 28, 2023
Merged

Conversation

sysvm
Copy link
Contributor

@sysvm sysvm commented Jun 24, 2023

  1. fix some comments errors such as: misspelling, syntax error and so on.
  2. extract a string const.

@sysvm sysvm force-pushed the docs/fixcomments branch 3 times, most recently from 39571d2 to 38e0e92 Compare June 27, 2023 13:11
// data: common p2p message data
func (n *Node) authenticateMessage(message proto.Message, data *p2p.MessageData) bool {
// store a temp ref to signature and remove it from message data
// sign is a string to allow easy reset to zero-value (empty string)
sign := data.Sign
data.Sign = nil

// marshall data without the signature to protobufs3 binary format
// marshall data without the signature to protobuf binary format
Copy link
Member

Choose a reason for hiding this comment

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

haven't looked but if it's actually marshaling to proto3 we may want to keep that distinction

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@@ -31,7 +31,7 @@ func expectHeader(r *bufio.Reader, expected []byte) error {
return err
}
if !bytes.Equal(header, expected) {
return fmt.Errorf("expected file header %s, got: %s", pathPSKv1, header)
return fmt.Errorf("expected file header %s, got: %s", expected, header)
Copy link
Member

Choose a reason for hiding this comment

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

Makes sense to change this to expected but I am not the expert here, cc @marten-seemann

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, this is correct, although in practice it doesn't make any difference, since this function is only called once with expected = pathPSKv1.

Copy link
Member

@p-shahi p-shahi left a comment

Choose a reason for hiding this comment

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

Thank you

@marten-seemann marten-seemann merged commit 8e341f7 into libp2p:master Jun 28, 2023
@sysvm sysvm deleted the docs/fixcomments branch June 29, 2023 02:25
@MarcoPolo MarcoPolo mentioned this pull request Jul 14, 2023
29 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.

4 participants