mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <>
To: Jonathan Nieder <>
Cc: Christian Couder <>,, Jeff King <>,
	Ben Peart <>,
	Jonathan Tan <>,
	Nguyen Thai Ngoc Duy <>,
	Mike Hommey <>,
	Lars Schneider <>,
	Eric Wong <>,
	Christian Couder <>
Subject: Re: [PATCH 1/2] Git/ rename packet_required_key_val_read()
Date: Wed, 22 Nov 2017 12:39:32 +0900	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <> (Jonathan Nieder's message of "Tue, 21 Nov 2017 11:19:00 -0800")

Jonathan Nieder <> writes:

> nit: please wrap lines to a consistent width, to make the message
> easier to read.  In the above, it looks like the line break is
> intentional --- is it meant to be two paragraphs (i.e. is it missing
> another newline)?

I'd think so; will add a missing LF while queuing..

> optional, just noticed while I'm nitpicking: the description 'rename
> packet_required_key_val_read' doesn't tell why the function is being
> renamed.  Maybe something like
> 	Git::Packet: clarify that packet_required_key_val_read allows EOF
> would do the trick.

Sounds good. 

>> +# Read a text line and check that it is in the form "key=value"
>> +sub packet_key_val_read {
> This comment doesn't tell me how to use the function.  How do I detect
> whether it successfully read a line?  What do the return values
> represent?  What happens if the line it read doesn't match the key?

Would this work for both of you?

# Read a text packet, expecting that it is in the form "key=value" for
# the given $key.  An EOF does not trigger any error and is reported
# back to the caller (like packet_txt_read() does).  Die if the "key"
# part of "key=value" does not match the given $key, or the value part
# is empty.

  reply	other threads:[~2017-11-22  3:39 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-21 16:09 [PATCH 1/2] Git/ rename packet_required_key_val_read() Christian Couder
2017-11-21 16:09 ` [PATCH 2/2] Git/ use 'if' instead of 'unless' Christian Couder
2017-11-21 19:24   ` Jonathan Nieder
2017-11-22  3:48     ` Junio C Hamano
2017-11-21 19:19 ` [PATCH 1/2] Git/ rename packet_required_key_val_read() Jonathan Nieder
2017-11-22  3:39   ` Junio C Hamano [this message]
2017-11-22  5:10     ` Jonathan Nieder
2017-11-22  6:28       ` Junio C Hamano
2017-11-22  7:07         ` Jonathan Nieder
2017-11-22  7:14         ` Christian Couder
2017-11-22  3:48 ` Junio C Hamano

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:

  List information:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \ \ \ \ \ \ \

* 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.
Code repositories for project(s) associated with this public inbox

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).