Subject: [quicwg/base-drafts] Improve description of max_ack_delay transport parameter (#1869)

From: Dmitri Tikhonov <[email-masked]>
Date: Wed, 17 Oct 2018 12:24:02 -0700

The sentence it replaces is awkward:

> An 8 bit unsigned integer value indicating the maximum amount of time in milliseconds by which _it_ will delay sending of acknowledgments.

What is _it_ -- the integer value?  Can an integer value delay sending of acknowledgements?
You can view, comment on, or merge this pull request online at:

  https://github.com/quicwg/base-drafts/pull/1869

-- Commit Summary --

  * Improve description of max_ack_delay transport parameter

-- File Changes --

    M draft-ietf-quic-transport.md (6)

-- Patch Links --

https://github.com/quicwg/base-drafts/pull/1869.patch
https://github.com/quicwg/base-drafts/pull/1869.diff

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/1869

---

From: ianswett <[email-masked]>
Date: Wed, 17 Oct 2018 12:40:19 -0700

ianswett commented on this pull request.

> @@ -1728,9 +1728,9 @@ disable_migration (0x0009):
 
 max_ack_delay (0x000c):
 
-: An 8 bit unsigned integer value indicating the maximum amount of time in
-  milliseconds by which it will delay sending of acknowledgments.  If this
-  value is absent, a default of 25 milliseconds is assumed.
+: An 8-bit unsigned integer value to use as the maximum delay (in milliseconds)

The intent of this transport param is for the receiver to communicate it's max ack delay to the sender, so I don't think the change from "indicating" to "to use as" is quite right.

How about "An 8-bit unsigned integer value indicating the maximum amount of time in milliseconds a receiver will delay sending an acknowledgement.  The receiver sends this value to the sender to set timeouts more accurately."?

I remember this was reworded a few times when I was writing it, 

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/1869#pullrequestreview-165802882

---

From: Dmitri Tikhonov <[email-masked]>
Date: Wed, 17 Oct 2018 12:52:24 -0700

dtikhonov commented on this pull request.

 max_ack_delay (0x000c):

Oh, that's right.  In this case, I how about "used as?"

> The maximum delay (in milliseconds) used to send acknowledgements encoded as an 8-bit unsigned integer.

Alternatively, I can close this PR and you can improve it as you see fit, @ianswett.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/1869#discussion_r226071814

---

From: Dmitri Tikhonov <[email-masked]>
Date: Wed, 17 Oct 2018 12:58:26 -0700

@dtikhonov pushed 1 commit.

0f5f6f2  Fix max_ack_delay: it's the *sender's* delay value

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1869/files/d19d65c108391f3229ecf5344860fd6df4ac2964..0f5f6f23a31a6c1cc846992d728d126469d80d0f

---

From: Martin Thomson <[email-masked]>
Date: Wed, 17 Oct 2018 14:42:08 -0700

martinthomson requested changes on this pull request.

 max_ack_delay (0x000c):
 
-: An 8 bit unsigned integer value indicating the maximum amount of time in
-  milliseconds by which it will delay sending of acknowledgments.  If this
-  value is absent, a default of 25 milliseconds is assumed.
+: The maximum delay (in milliseconds) used to send acknowledgements, encoded as
+  an 8-bit unsigned integer.  If this value is absent, a default of 25

Nor do you use a delay to send acknowledgments.  Why not simply s/it/the endpoint/ ?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/1869#pullrequestreview-165849154

---

From: Dmitri Tikhonov <[email-masked]>
Date: Thu, 18 Oct 2018 05:24:38 -0700

max_ack_delay (0x000c):

OK, this is good.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/1869#discussion_r226282932

---

From: Dmitri Tikhonov <[email-masked]>
Date: Thu, 18 Oct 2018 05:26:51 -0700

@dtikhonov pushed 3 commits.

ab99715  Revert "Fix max_ack_delay: it's the *sender's* delay value"
0acc50b  Revert "Improve description of max_ack_delay transport parameter"
b63e386  Use Martin Thomson's suggestion to improve max_ack_delay description

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1869/files/0f5f6f23a31a6c1cc846992d728d126469d80d0f..b63e386e8763aaf4fc780d37dee92b22ac038d60

---

From: ianswett <[email-masked]>
Date: Thu, 18 Oct 2018 13:51:37 +0000 (UTC)

> @@ -1729,8 +1729,8 @@ disable_migration (0x0009):
 max_ack_delay (0x000c):
 
 : An 8 bit unsigned integer value indicating the maximum amount of time in
-  milliseconds by which it will delay sending of acknowledgments.  If this
-  value is absent, a default of 25 milliseconds is assumed.
+  milliseconds by which the endpoint will delay sending of acknowledgments.

nit: I think "sending acknowledgements." reads a bit better than "sending of acknowledgments."

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/1869#pullrequestreview-166103495

---

From: Dmitri Tikhonov <[email-masked]>
Date: Thu, 18 Oct 2018 06:55:35 -0700

f2ba00b  s/sending of acknowledgments/sending acknowledgments/

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1869/files/b63e386e8763aaf4fc780d37dee92b22ac038d60..f2ba00bcaaac62ee18754ad61f79f666a6b82967

---

From: ianswett <[email-masked]>
Date: Thu, 18 Oct 2018 06:56:36 -0700

ianswett approved this pull request.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/1869#pullrequestreview-166106194

---

From: Martin Thomson <[email-masked]>
Date: Thu, 18 Oct 2018 17:19:57 -0700

Closed #1869 via 18a7c3e6bf243ba41328ffb11361c49da3c4e173.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/1869#event-1913638906

---

From: Martin Thomson <[email-masked]>
Date: Thu, 18 Oct 2018 17:20:10 -0700

needed a manual merge after the big reorg

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/1869#issuecomment-431205492