Re: [PATCH v2 05/18] fsck: Allow demoting errors to warnings via receive.fsck.warn = <key>
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Jan 21, 2015, 18:01 UTC
- Message-ID
- <07b285ef702031fdebcf9def59528b9c@www.dscho.org>
- In-Reply-To
- <xmqqlhkwy0my.fsf@gitster.dls.corp.google.com>
Hi Junio,
On 2015-01-21 09:54, Junio C Hamano wrote:
Show 10 quoted lines
> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
>
>>
>> + if (starts_with(var, "receive.fsck.")) {
>> + if (fsck_severity.len)
>> + strbuf_addch(&fsck_severity, ',');
>> + strbuf_addf(&fsck_severity, "%s=%s", var + 13, value);
>
> Wouldn't it be safer to use skip_prefix() that lets you avoid the
> hardcoded "var + 13" here?Yep, and much more elegant, too. I also fixed three more instances of the same pattern.
Show 32 quoted lines
>> @@ -1470,8 +1478,13 @@ static const char *unpack(int err_fd, struct shallow_info *si)
>> argv_array_pushl(&child.args, "unpack-objects", hdr_arg, NULL);
>> if (quiet)
>> argv_array_push(&child.args, "-q");
>> - if (fsck_objects)
>> - argv_array_push(&child.args, "--strict");
>> + if (fsck_objects) {
>> + if (fsck_severity.len)
>> + argv_array_pushf(&child.args, "--strict=%s",
>> + fsck_severity.buf);
>> + else
>> + argv_array_push(&child.args, "--strict");
>> + }
>> child.no_stdout = 1;
>> child.err = err_fd;
>> child.git_cmd = 1;
>> @@ -1488,8 +1501,13 @@ static const char *unpack(int err_fd, struct shallow_info *si)
>>
>> argv_array_pushl(&child.args, "index-pack",
>> "--stdin", hdr_arg, keep_arg, NULL);
>> - if (fsck_objects)
>> - argv_array_push(&child.args, "--strict");
>> + if (fsck_objects) {
>> + if (fsck_severity.len)
>> + argv_array_pushf(&child.args, "--strict=%s",
>> + fsck_severity.buf);
>> + else
>> + argv_array_push(&child.args, "--strict");
>> + }
>
> Hmm. The above two hunks look suspiciously similar. Would it be
> worth to give them a single helper function?Hmm. Not sure. I see what you mean, but for now I found
+ argv_array_pushf(&child.args, "--strict%s%s", + fsck_severity.len ? "=" : "", + fsck_severity.buf);
to be more elegant than to add a fully-fledged new function. But if you feel strongly, I will gladly implement a separate function; I would appreciate suggestions as to the function name...
Ciao, Dscho