Subject: Re: [PATCH v5 06/10] test-http-server: add simple authentication

From: Junio C Hamano <[email-masked]>
Date: Fri, 13 Jan 2023 13:06:07 -0800

Victoria Dye <[email-masked]> writes:

>> +static int split_auth_param(const char *str, char **scheme, char **val, int required_val)
>> +{
>> ...
>> +}
>
> There's nothing really *new* in these functions in this iteration, just code
> moved from the option parsing/handling in 'cmd_main()' into dedicated
> functions. Looks good!

> ...
>
> I completely missed the "fall-through" comment in my last review [1], as you
> kindly pointed out [2]. ;) Given that, this makes sense to me.

>> +		*wr = send_http_error(STDOUT_FILENO, 401, "Unauthorized", -1,
>> +				      &hdrs, *wr);
>
> The "extra_headers" configuration is new, and helps make the test server
> more flexible. 

This is not limited to this single review message, but it is good to
see "this part of the patch is good because ..." explicitly stated.
I wish more people did so, in addition to pointing out what needs to
be improved.

Thanks.