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

Refer to all header octets by number #651

Merged
merged 4 commits into from Jun 29, 2017
Merged

Refer to all header octets by number #651

merged 4 commits into from Jun 29, 2017

Conversation

martinthomson
Copy link
Member

Closes #580

@martinthomson martinthomson added -transport editorial An issue that does not affect the design of the protocol; does not require consensus. labels Jun 21, 2017
@martinthomson martinthomson changed the title Refer to all octets by number Refer to all header octets by number Jun 21, 2017
Copy link
Contributor

@ianswett ianswett left a comment

Choose a reason for hiding this comment

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

Two small suggestions.

@@ -461,24 +461,24 @@ This header form has the following fields:

Header Form:

: The most significant bit (0x80) of the first octet of a packet is the header
: The most significant bit (0x80) of the octet 0 of a packet is the header
Copy link
Contributor

Choose a reason for hiding this comment

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

This text is a bit redundant to the "Header Form" section above, and the sentence is a bit awkward. Can you just say "The most significant bit (0x80) of octet 0 is set to 0 for the short header."?

form. This bit is set to 0 for the short header.

Connection ID Flag:

: The second bit (0x40) of the first octet indicates whether the Connection ID
: The second bit (0x40) of the octet 0 indicates whether the Connection ID
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I'd change "the octet 0" to "octet 0", here and below.

Copy link
Contributor

@ianswett ianswett left a comment

Choose a reason for hiding this comment

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

One more nit, but looks good.


Long Packet Type:

: The remaining seven bits of first octet of a long packet is the packet type.
This field can indicate one of 128 packet types. The types specified for this
: The remaining seven bits of octet 0 of a long packet is the packet type. This
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: is => are?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think "of a long packet" is redundant given "Long Packet Type:" is the title as well, and it makes the sentence a bit awkward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-transport editorial An issue that does not affect the design of the protocol; does not require consensus.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants