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.

Sign up for GitHub

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

Jump to bottom

priority: use SF terms, not ABNF #1977

Merged
merged 4 commits into from
Apr 6, 2022
Merged

priority: use SF terms, not ABNF #1977

merged 4 commits into from
Apr 6, 2022

Conversation

LPardue
Copy link
Contributor

@LPardue LPardue commented Feb 17, 2022

Closes #1975.

We're not in AUTH48 yet. I suggest we hold merging the change until that time.

@LPardue LPardue requested a review from kazuho as a code owner February 17, 2022 01:20
martinthomson
martinthomson reviewed Feb 17, 2022
View reviewed changes
draft-ietf-httpbis-priority.md Outdated
@@ -130,8 +130,8 @@ consideration and guidance about how servers might act upon signals.

{::boilerplate bcp14-tagged}

The terms Dictionary, sf-boolean, sf-dictionary, and sf-integer are imported from
{{!STRUCTURED-FIELDS=RFC8941}}.
The terms Boolean, Dictionary, and Integer, along with the sf-dictionary rule,
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think that you want to say that RFC 8941 defines the term "Boolean". Maybe instead "This document uses the Boolean, Dictionary, and Integer types from {{!SF}} along with the sf-dictionary ABNF rule."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. "Boolean" is so generic, it was part of the reason why I gravitated to sf-boolean in prose in the first place.

Copy link
Member

Choose a reason for hiding this comment

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

In prose, you can try "Boolean Structured Type" or similar.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The pattern I've seen so far is people saying "Structured Fields Dictionary" or "Structured Fields List". So "Structured Fields Boolean" would be consistent with that but its a bit wordy. Perhaps settling on a common acronym of SF in front of these would work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Applying this to "SF Member" would also be help in specs that need to talk generically about the things in List or Dictionary without wanting to have to make up a term of their own.

ioggstream
ioggstream reviewed Feb 18, 2022
View reviewed changes
draft-ietf-httpbis-priority.md Outdated
@@ -299,7 +299,8 @@ ignored.
The urgency parameter (`u`) takes an integer between 0 and 7, in descending
order of priority.

The value is encoded as an sf-integer. The default value is 3.
The value is encoded as an Integer (see {{Section 3.3.1 of STRUCTURED-FIELDS}}).
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
The value is encoded as an Integer (see {{Section 3.3.1 of STRUCTURED-FIELDS}}).
The value is an Integer (see {{Section 3.3.1 of STRUCTURED-FIELDS}}).

encoded reminds me of syntaxes :P

ioggstream
ioggstream reviewed Feb 18, 2022
View reviewed changes
draft-ietf-httpbis-priority.md Outdated
Comment on lines 332 to 333
The incremental parameter (`i`) takes a Boolean (see
{{Section 3.3.6 of STRUCTURED-FIELDS}}) as the value that indicates
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
The incremental parameter (`i`) takes a Boolean (see
{{Section 3.3.6 of STRUCTURED-FIELDS}}) as the value that indicates
The incremental parameter (`i`) is a Boolean (see
{{Section 3.3.6 of STRUCTURED-FIELDS}}) that indicates

ioggstream
ioggstream reviewed Feb 18, 2022
View reviewed changes
Copy link
Contributor

@ioggstream ioggstream left a comment

Choose a reason for hiding this comment

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

@LPardue Just to make some SF-practice. Let me know :) R.

@LPardue LPardue changed the base branch from main to priority-auth48-all March 28, 2022 23:39
@LPardue LPardue merged commit d3beb9a into priority-auth48-all Apr 6, 2022
@LPardue LPardue deleted the lucas/priority-sf-terms branch April 6, 2022 22:53
LPardue added a commit that referenced this pull request Jun 9, 2022
@LPardue
priority: use SF terms, not ABNF (#1977)
10857dc 
* priority: use SF terms, not ABNF
LPardue added a commit that referenced this pull request Jun 10, 2022
@LPardue @martinthomson @kazuho
All the priorities auth 48 changes (#2154)
7230dfd 
* uncontroversial AUTH48 (#2044)
* priority: update author name (#1978)

* Update author name

kramdown-rfc2629 v 1.5.26 added the ability for non-ascii names
in markdown that don't generate XML that xml2rfc would reject.


Co-authored-by: Martin Thomson <mt@lowentropy.net>

* always RFC stream priorities

* IANA considerations style fixups (#2050)

* streams carry requests, not methods (#2052)

* couple of IANA fixups

* priority parameters are lowercase

* lowercase Stream and Push ID in prose

* Priority Field Value

* false is <tt> too

* Tweak the acknowledgements

* Add priority keywords (#2056)

* rephrase multiplexing speak on H1.1 and older (#2055)

* Frame defs and examples are art (#2054)

* add clarifying statements about stream state outcomes (#2051)

* s/Specification:/Reference:

* totally after is totally lame

* you consult a doctor, not an HTTP request

* tweak contrast of frames and headers

* their values -> its value

* avoiding -> preventing

* CDN is actually uppercase

* fix external refs

* remove changelog

* benefit sending -> benefit in sending

* one more tt for true

* H2 frame format

* H2 frame format comma not dot

* apparently all i.e. need a comma

* either -> in either of

* Prioritized stream -> prioritized stream

* open -> open state

* no comma here!

* align refs with HTTP style (#2057)

* priority: use SF terms, not ABNF (#1977)

* priority: use SF terms, not ABNF

* Tweak IANA stuff (#2059)

Closes #2006

A parameter by any name would be sweet

* Try to avoid saying payload where it is not needed (#2060)

* remove superfluous description of H2 reserved field

* auth48 tiny editorial nits (#2067)

* s/Structured Fields Dictionary/Dictionary/

* either of the "half-closed" state(s)

* was originally "the CONNECT method"

* Intermediary signal processing/production, again (#2058)

Closes #2003

* State even more clearly intermediary unfairness (#2053)

* State even more clearly intermediary unfairness

Co-authored-by: Kazuho Oku <kazuhooku@gmail.com>

* Add space to author name

Co-authored-by: Martin Thomson <mt@lowentropy.net>
Co-authored-by: Kazuho Oku <kazuhooku@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@martinthomson martinthomson martinthomson left review comments

@mnot mnot mnot left review comments

@ioggstream ioggstream ioggstream left review comments

@kazuho kazuho Awaiting requested review from kazuho

Assignees
No one assigned
Labels
priorities
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Use SF terms, not ABNF
4 participants
@LPardue @martinthomson @mnot @ioggstream