improve description of version field in traceresponse header #512
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This fixes the confusing wording in the description of the version field, which previously didn't clearly distinguish between a possible internal representation (1 byte) and the format on the wire (two ascii chars). This change ports the new description of the version field in the request header over to the response header and aligns both descriptions. See w3c#511.
basti1302
force-pushed
the
fix-response-header-version-field
branch
from
af1d9b7 to
ab82e2b
Compare
basti1302
changed the title
basti1302
added
the
Editorial
kalyanaj
approved these changes
Dec 5, 2022
dyladan
approved these changes
Dec 6, 2022
| The value is US-ASCII encoded (which is UTF-8 compliant). | ||
|
|
||
| Version (`version`) is 1 byte representing an 8-bit unsigned integer. Version `255` is invalid. The current specification assumes the `version` is set to `00`. | ||
| Version (`version`) is an 8-bit unsigned integer value, serialized as an ASCII string with two characters. Version 255 (`"ff"`) is invalid. This specification assumes the version is set to 0 (`"00"`). |
There was a problem hiding this comment.
Do we "assume" version is set to 00 or do we require it? We can keep the existing wording if it is easier but I think something like this is more accurate.
Suggested change
| Version (`version`) is an 8-bit unsigned integer value, serialized as an ASCII string with two characters. Version 255 (`"ff"`) is invalid. This specification assumes the version is set to 0 (`"00"`). | |
| Version (`version`) is an 8-bit unsigned integer value, serialized as an ASCII string with two characters. Version 255 (`"ff"`) is invalid. This document specifies version 0 (`"00"`) of the traceparent header. |
There was a problem hiding this comment.
Good point. This section is about the traceresponse header though. I'll fix that in a follow up commit.
There was a problem hiding this comment.
(For context, this is not yet the backport to level-2, I just ported the changes for traceparent also to the same fields for traceparent.)
There was a problem hiding this comment.
For the record, I commited this change and then rewrote the commit to say of the traceresponse header.
basti1302
force-pushed
the
fix-response-header-version-field
branch
from
f740391 to
c9e06dd
Compare
Co-authored-by: Daniel Dyla <dyladan@users.noreply.github.com>
basti1302
force-pushed
the
fix-response-header-version-field
branch
from
c9e06dd to
ad4d9d0
Compare
dyladan
approved these changes
Dec 7, 2022
|
For the record: Since this is for the response header, it is not applicable to be backported to level 2. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes the confusing wording in the description of the version field, which previously didn't clearly distinguish between a possible internal representation (1 byte) and the format on the wire (two ascii chars).
This change ports the new description of the version field in the request header over to the response header and aligns both descriptions. See #511.
In contrast to #511, this will not be backported to level 2 (level 2 does not have the response header).
Preview | Diff