Jonathan Nieder <jrnie...@gmail.com> 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.
