From: "Jakub Narębski" <jnareb@gmail.com>
To: Lars Schneider <larsxschneider@gmail.com>, git@vger.kernel.org
Cc: "Jeff King" <peff@peff.net>, "Junio C Hamano" <gitster@pobox.com>,
"Stefan Beller" <sbeller@google.com>,
"Martin-Louis Bright" <mlbright@gmail.com>,
"Torsten Bögershausen" <tboegi@web.de>,
"Ramsay Jones" <ramsay@ramsayjones.plus.com>
Subject: Re: [PATCH v8 01/11] pkt-line: rename packet_write() to packet_write_fmt()
Date: Sat, 24 Sep 2016 23:14:36 +0200 [thread overview]
Message-ID: <592e18b7-ac2e-ed23-0321-57eb418d5172@gmail.com> (raw)
In-Reply-To: <20160920190247.82189-2-larsxschneider@gmail.com>
Hello Lars,
W dniu 20.09.2016 o 21:02, larsxschneider@gmail.com pisze:
> From: Lars Schneider <larsxschneider@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 <gitster@pobox.com>
Just so nobody wonders later why this patch was needed/suggested.
> Signed-off-by: Lars Schneider <larsxschneider@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
next prev parent reply other threads:[~2016-09-24 21:15 UTC|newest]
Thread overview: 71+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-20 19:02 [PATCH v8 00/11] Git filter protocol larsxschneider
2016-09-20 19:02 ` [PATCH v8 01/11] pkt-line: rename packet_write() to packet_write_fmt() larsxschneider
2016-09-24 21:14 ` Jakub Narębski [this message]
2016-09-26 18:49 ` Lars Schneider
2016-09-28 23:15 ` Jakub Narębski
2016-09-20 19:02 ` [PATCH v8 02/11] pkt-line: extract set_packet_header() larsxschneider
2016-09-24 21:22 ` Jakub Narębski
2016-09-26 18:53 ` Lars Schneider
2016-09-20 19:02 ` [PATCH v8 03/11] run-command: move check_pipe() from write_or_die to run_command larsxschneider
2016-09-24 22:12 ` Jakub Narębski
2016-09-26 16:13 ` Lars Schneider
2016-09-26 16:21 ` Jakub Narębski
2016-09-20 19:02 ` [PATCH v8 04/11] pkt-line: add packet_write_fmt_gently() larsxschneider
2016-09-24 22:27 ` Jakub Narębski
2016-09-20 19:02 ` [PATCH v8 05/11] pkt-line: add packet_flush_gently() larsxschneider
2016-09-24 22:56 ` Jakub Narębski
2016-09-20 19:02 ` [PATCH v8 06/11] pkt-line: add packet_write_gently() larsxschneider
2016-09-25 11:26 ` Jakub Narębski
2016-09-26 19:21 ` Lars Schneider
2016-09-27 8:39 ` Jeff King
2016-09-27 19:33 ` Jakub Narębski
2016-09-20 19:02 ` [PATCH v8 07/11] pkt-line: add functions to read/write flush terminated packet streams larsxschneider
2016-09-25 13:46 ` Jakub Narębski
2016-09-26 20:23 ` Lars Schneider
2016-09-27 8:14 ` Lars Schneider
2016-09-27 9:00 ` Jeff King
2016-09-27 12:10 ` Lars Schneider
2016-09-27 12:13 ` Jeff King
2016-09-20 19:02 ` [PATCH v8 08/11] convert: quote filter names in error messages larsxschneider
2016-09-25 14:03 ` Jakub Narębski
2016-09-20 19:02 ` [PATCH v8 09/11] convert: modernize tests larsxschneider
2016-09-25 14:43 ` Jakub Narębski
2016-09-20 19:02 ` [PATCH v8 10/11] convert: make apply_filter() adhere to standard Git error handling larsxschneider
2016-09-25 14:47 ` Jakub Narębski
2016-09-20 19:02 ` [PATCH v8 11/11] convert: add filter.<driver>.process option larsxschneider
2016-09-26 22:41 ` Jakub Narębski
2016-09-30 18:56 ` Lars Schneider
2016-10-04 20:50 ` Jakub Narębski
2016-10-06 13:16 ` Lars Schneider
2016-09-27 15:37 ` Jakub Narębski
2016-09-30 19:38 ` Lars Schneider
2016-10-04 21:00 ` Jakub Narębski
2016-10-06 21:27 ` Lars Schneider
2016-09-28 23:14 ` Jakub Narębski
2016-10-01 15:34 ` Lars Schneider
2016-10-04 21:34 ` Jakub Narębski
2016-09-28 21:49 ` [PATCH v8 00/11] Git filter protocol Junio C Hamano
2016-09-29 10:28 ` Lars Schneider
2016-09-29 11:57 ` Torsten Bögershausen
2016-09-29 16:57 ` Junio C Hamano
2016-09-29 17:57 ` Lars Schneider
2016-09-29 18:18 ` Torsten Bögershausen
2016-09-29 18:38 ` Johannes Sixt
2016-09-29 21:27 ` Junio C Hamano
2016-10-01 18:59 ` Lars Schneider
2016-10-01 20:48 ` Jakub Narębski
2016-10-03 17:13 ` Lars Schneider
2016-10-04 19:04 ` Jakub Narębski
2016-10-06 13:13 ` Lars Schneider
2016-10-06 16:01 ` Jeff King
2016-10-06 17:17 ` Junio C Hamano
2016-10-03 17:02 ` Junio C Hamano
2016-10-03 17:35 ` Lars Schneider
2016-10-04 12:11 ` Jeff King
2016-10-04 16:47 ` Junio C Hamano
2016-09-29 18:02 ` Jeff King
2016-09-29 21:19 ` Junio C Hamano
2016-09-29 20:50 ` Lars Schneider
2016-09-29 21:12 ` Junio C Hamano
2016-09-29 20:59 ` Jakub Narębski
2016-09-29 21:17 ` 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:
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=592e18b7-ac2e-ed23-0321-57eb418d5172@gmail.com \
--to=jnareb@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=larsxschneider@gmail.com \
--cc=mlbright@gmail.com \
--cc=peff@peff.net \
--cc=ramsay@ramsayjones.plus.com \
--cc=sbeller@google.com \
--cc=tboegi@web.de \
/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;
as well as URLs for NNTP newsgroup(s).