Date: Wed, 22 Jan 2020 14:37:03 -0800
From: Martin Thomson <notifications@github.com>
Reply-To: quicwg/base-drafts
 <reply+AFTOJK3NWGKAK3HW2RXEMWV4GYAQ7EVBNHHCCABNRQ@reply.github.com>
To: quicwg/base-drafts <base-drafts@noreply.github.com>
Cc: Subscribed <subscribed@noreply.github.com>
Message-ID: <quicwg/base-drafts/pull/3383/review/346952994@github.com>
In-Reply-To: <quicwg/base-drafts/pull/3383@github.com>
References: <quicwg/base-drafts/pull/3383@github.com>
Subject: Re: [quicwg/base-drafts] editorial: This sentence was hard to parse
 for me (recovery) (#3383)
Mime-Version: 1.0
Content-Type: multipart/alternative;
 boundary="--==_mimepart_5e28ce8f2f71d_7acc3fb5916cd9609294";
 charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: list
Archived-At: <https://mailarchive.ietf.org/arch/msg/quic-issues/o7cIlU8avIGxxddqMPinLMSmJvQ>


----==_mimepart_5e28ce8f2f71d_7acc3fb5916cd9609294
Content-Type: text/plain;
 charset=UTF-8
Content-Transfer-Encoding: 7bit

martinthomson approved this pull request.

This is a great improvement, thanks.

One additional suggestion (for existing text) below.

> @@ -272,9 +272,10 @@ SHOULD NOT be used to update RTT estimates if it does not newly acknowledge the
 largest acknowledged packet.
 
 An RTT sample MUST NOT be generated on receiving an ACK frame that does not
-newly acknowledge at least one ack-eliciting packet.  A peer does not send an
-ACK frame on receiving only non-ack-eliciting packets, so an ACK frame that is
-subsequently sent can include an arbitrarily large Ack Delay field.  Ignoring
+newly acknowledge at least one ack-eliciting packet. A peer usually does not
+send an ACK frame when only non-ack-eliciting packets are received. Therefore
+an ACK frame that only contains acknowledgements for non-ack-eliciting packets
+can subsequently include an arbitrarily large Ack Delay value.  Ignoring

```suggestion
could include an arbitrarily large Ack Delay value.  Ignoring
```

-- 
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/3383#pullrequestreview-346952994
----==_mimepart_5e28ce8f2f71d_7acc3fb5916cd9609294
Content-Type: text/html;
 charset=UTF-8
Content-Transfer-Encoding: quoted-printable

<p><b>@martinthomson</b> approved this pull request.</p>=0D
=0D
<p>This is a great improvement, thanks.</p>=0D
<p>One additional suggestion (for existing text) below.</p><hr>=0D
=0D
<p>In <a href=3D"https://github.com/quicwg/base-drafts/pull/3383#discussi=
on_r369843317">draft-ietf-quic-recovery.md</a>:</p>=0D
<pre style=3D'color:#555'>&gt; @@ -272,9 +272,10 @@ SHOULD NOT be used to=
 update RTT estimates if it does not newly acknowledge the=0D
 largest acknowledged packet.=0D
 =0D
 An RTT sample MUST NOT be generated on receiving an ACK frame that does =
not=0D
-newly acknowledge at least one ack-eliciting packet.  A peer does not se=
nd an=0D
-ACK frame on receiving only non-ack-eliciting packets, so an ACK frame t=
hat is=0D
-subsequently sent can include an arbitrarily large Ack Delay field.  Ign=
oring=0D
+newly acknowledge at least one ack-eliciting packet. A peer usually does=
 not=0D
+send an ACK frame when only non-ack-eliciting packets are received. Ther=
efore=0D
+an ACK frame that only contains acknowledgements for non-ack-eliciting p=
ackets=0D
+can subsequently include an arbitrarily large Ack Delay value.  Ignoring=
=0D
</pre>=0D
=E2=AC=87=EF=B8=8F Suggested change=0D
<pre style=3D"color: #555">-can subsequently include an arbitrarily large=
 Ack Delay value.  Ignoring=0D
+could include an arbitrarily large Ack Delay value.  Ignoring=0D
</pre>=0D
=0D
=0D
<p style=3D"font-size:small;-webkit-text-size-adjust:none;color:#666;">&m=
dash;<br />You are receiving this because you are subscribed to this thre=
ad.<br />Reply to this email directly, <a href=3D"https://github.com/quic=
wg/base-drafts/pull/3383?email_source=3Dnotifications&amp;email_token=3DA=
FTOJKYP6KH6QW6UHJV452DQ7DDA7A5CNFSM4KKI3SV2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJ=
SXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCSXBKIQ#pullrequestreview-346952994=
">view it on GitHub</a>, or <a href=3D"https://github.com/notifications/u=
nsubscribe-auth/AFTOJK46O3SPBLMVMTOYBO3Q7DDA7ANCNFSM4KKI3SVQ">unsubscribe=
</a>.<img src=3D"https://github.com/notifications/beacon/AFTOJK6YYWDDIAA7=
4UIF7BTQ7DDA7A5CNFSM4KKI3SV2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWN=
FSXPKTDN5WW2ZLOORPWSZGOCSXBKIQ.gif" height=3D"1" width=3D"1" alt=3D"" /><=
/p>=0D
<script type=3D"application/ld+json">[=0D
{=0D
"@context": "http://schema.org",=0D
"@type": "EmailMessage",=0D
"potentialAction": {=0D
"@type": "ViewAction",=0D
"target": "https://github.com/quicwg/base-drafts/pull/3383?email_source=3D=
notifications\u0026email_token=3DAFTOJKYP6KH6QW6UHJV452DQ7DDA7A5CNFSM4KKI=
3SV2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOC=
SXBKIQ#pullrequestreview-346952994",=0D
"url": "https://github.com/quicwg/base-drafts/pull/3383?email_source=3Dno=
tifications\u0026email_token=3DAFTOJKYP6KH6QW6UHJV452DQ7DDA7A5CNFSM4KKI3S=
V2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCSX=
BKIQ#pullrequestreview-346952994",=0D
"name": "View Pull Request"=0D
},=0D
"description": "View this Pull Request on GitHub",=0D
"publisher": {=0D
"@type": "Organization",=0D
"name": "GitHub",=0D
"url": "https://github.com"=0D
}=0D
}=0D
]</script>=

----==_mimepart_5e28ce8f2f71d_7acc3fb5916cd9609294--