From: Denton Liu <liu.denton@gmail.com>
To: Joey Salazar <jgsal@protonmail.com>
Cc: "git@vger.kernel.org" <git@vger.kernel.org>
Subject: Re: [OUTREACHY][PATCH v1] doc: fix naming of response-end-pkt
Date: Wed, 17 Feb 2021 03:07:39 -0800 [thread overview]
Message-ID: <YCz4+xPVyL+hTZzR@generichostname> (raw)
In-Reply-To: <5qGT6uzPLUGN2DXCMTzhixEhKHwaT6hODaOHQ485sfCROycrTDPx6P2Nd5dOy4J-gnhb_lKpxW4jJqhut-4gmoeIyuhpqbA5fXCeHoKHrK8=@protonmail.com>
Hi Joey,
On Tue, Feb 16, 2021 at 09:21:50PM +0000, Joey Salazar wrote:
> Git Protocol version 2[1] defines 0002 as a Message Packet that indicates
> the end of a response for stateless connections.
>
> Change the naming of the 0002 Packet to 'Response end' to match the
> parsing introduced in Wireshark's MR !1922 for consistency.
Thanks for catching this, this is an obvious error on my part. In fact,
I'd go as far as saying that in the commit where this was defined,
b0df0c16 (stateless-connect: send response end packet, 2020-05-19), I
erroneously called it a "Message Packet" when I meant to type "Response
End Packet", which you are now correcting.
> [1] kernel.org/pub/software/scm/git/docs/technical/protocol-v2.html
> [2] gitlab.com/wireshark/wireshark/-/merge_requests/1922
>
> Signed-off-by: Joey Salazar <jgsal@protonmail.com>
> ---
> Documentation/technical/protocol-v2.txt | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/technical/protocol-v2.txt b/Documentation/technical/protocol-v2.txt
> index e597b74da39..6c55d566d8b 100644
> --- a/Documentation/technical/protocol-v2.txt
> +++ b/Documentation/technical/protocol-v2.txt
> @@ -33,8 +33,8 @@ In protocol v2 these special packets will have the following semantics:
>
> * '0000' Flush Packet (flush-pkt) - indicates the end of a message
> * '0001' Delimiter Packet (delim-pkt) - separates sections of a message
> - * '0002' Message Packet (response-end-pkt) - indicates the end of a response
> - for stateless connections
> + * '0002' Response end Packet (response_end-pkt) - indicates the end of a
> + response for stateless connections
A couple of aesthetic details: I see that these changes are based on MR
!1922 so if you decide to take these suggestions, you'll probably need
to apply them there too.
1. It'd probably read better as "Response End Packet" (with the
"e" capitalised)
2. The mix of underscore and hyphen in response_end-pkt is a
little odd, although I see that you've done it to make it
match the surrounding code[3].
I think that I'd prefer if 1. is taken and 2. can be ignored. But with
or without those changes, consider this patch
Acked-by: Denton Liu <liu.denton@gmail.com>
Thanks,
Denton
[3]: https://gitlab.com/wireshark/wireshark/-/merge_requests/1922#note_502316230
> Initial Client Request
> ----------------------
> --
> 2.29.0.rc2
>
next prev parent reply other threads:[~2021-02-17 11:08 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-16 21:21 [OUTREACHY][PATCH v1] doc: fix naming of response-end-pkt Joey Salazar
2021-02-16 23:03 ` Junio C Hamano
2021-02-17 11:07 ` Denton Liu [this message]
2021-02-17 23:59 ` Joey Salazar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YCz4+xPVyL+hTZzR@generichostname \
--to=liu.denton@gmail.com \
--cc=git@vger.kernel.org \
--cc=jgsal@protonmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox