Hello Lars,

W dniu 20.09.2016 o 21:02, larsxschnei...@gmail.com pisze:

> From: Lars Schneider <larsxschnei...@gmail.com>
> 
> packet_write() should be called packet_write_fmt() as the string
> parameter can be formatted.

I would say:

  packet_write() should be called packet_write_fmt() because it
  is printf-like function where first parameter is format string.
  
Or something like that.  But such minor change might be not worth
yet another reroll of this patch series.

Perhaps it would be a good idea to explain the reasoning behind
this change:

  This is important distinction to know from the name if the
  function accepts arbitrary binary data and/or arbitrary
  strings to be written - packet_write[_fmt()] do not.

> 
> Suggested-by: Junio C Hamano <gits...@pobox.com>

Just so nobody wonders later why this patch was needed/suggested.

> Signed-off-by: Lars Schneider <larsxschnei...@gmail.com>
> ---
>  builtin/archive.c        |  4 ++--
>  builtin/receive-pack.c   |  4 ++--
>  builtin/remote-ext.c     |  4 ++--
>  builtin/upload-archive.c |  4 ++--
>  connect.c                |  2 +-
>  daemon.c                 |  2 +-
>  http-backend.c           |  2 +-
>  pkt-line.c               |  2 +-

The header of the renamed function looks now very nice:

 void packet_write_fmt(int fd, const char *fmt, ...)
                   ^^^                     ^^^

>  pkt-line.h               |  2 +-
>  shallow.c                |  2 +-
>  upload-pack.c            | 30 +++++++++++++++---------------
>  11 files changed, 29 insertions(+), 29 deletions(-)

Diffstat looks correct.  Was the patch generated by doing search
and replace?

Best,
-- 
Jakub Narębski

Reply via email to