Re: [PATCH 1/2] Git/Packet.pm: rename packet_required_key_val_read()

2017-11-21 Thread Christian Couder
On Wed, Nov 22, 2017 at 7:28 AM, Junio C Hamano wrote: > Jonathan Nieder writes: > 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

Re: [PATCH 1/2] Git/Packet.pm: rename packet_required_key_val_read()

2017-11-21 Thread Jonathan Nieder
Junio C Hamano wrote: > Jonathan Nieder writes: >> Junio C Hamano wrote: >>> Jonathan Nieder writes: 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

Re: [PATCH 1/2] Git/Packet.pm: rename packet_required_key_val_read()

2017-11-21 Thread Junio C Hamano
Jonathan Nieder writes: >>> 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

Re: [PATCH 1/2] Git/Packet.pm: rename packet_required_key_val_read()

2017-11-21 Thread Jonathan Nieder
Junio C Hamano wrote: > Jonathan Nieder writes: >> Christian Couder wrote: >>> +# 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

Re: [PATCH 1/2] Git/Packet.pm: rename packet_required_key_val_read()

2017-11-21 Thread Junio C Hamano
Christian Couder writes: > The function calls itself "required", but it does not die when it > sees an unexpected EOF. > Let's rename it to "packet_key_val_read()". > > Signed-off-by: Christian Couder > --- > > These 2 patches are a late

Re: [PATCH 1/2] Git/Packet.pm: rename packet_required_key_val_read()

2017-11-21 Thread Junio C Hamano
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

Re: [PATCH 1/2] Git/Packet.pm: rename packet_required_key_val_read()

2017-11-21 Thread Jonathan Nieder
Hi, Christian Couder wrote: > The function calls itself "required", but it does not die when it > sees an unexpected EOF. > Let's rename it to "packet_key_val_read()". > > Signed-off-by: Christian Couder > --- nit: please wrap lines to a consistent width, to make the

[PATCH 1/2] Git/Packet.pm: rename packet_required_key_val_read()

2017-11-21 Thread Christian Couder
The function calls itself "required", but it does not die when it sees an unexpected EOF. Let's rename it to "packet_key_val_read()". Signed-off-by: Christian Couder --- These 2 patches are a late follow up from: