Thread (1 message) 1 message, 1 author, 2023-01-13

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

From: Junio C Hamano <hidden>
Date: 2023-01-13 21:06:14

Victoria Dye [off-list ref] writes:
quoted
+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.
quoted
+		*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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help