Re: [PATCH 1/2] http: allow selection of proxy authentication method
- From
- Knut Franke <k.franke@science-computing.de>
- Date
- Nov 3, 2015, 09:07 UTC
- Message-ID
- <20151103090724.GA6354@science-computing.de>
- In-Reply-To
- <xmqqfv0odnoc.fsf@gitster.mtv.corp.google.com>
On 2015-11-02 14:46, Junio C Hamano wrote:
Show 7 quoted lines
> > Reviewed-by: Junio C Hamano <gitster@pobox.com> > > Reviewed-by: Eric Sunshine <sunshine@sunshineco.com> > > Please add these only when you are doing the final submission, > sending the same version reviewed by these people after they said > the patch(es) look good. To credit others for helping you to polish > your patch, Helped-by: would be more appropriate.
Sorry about that.
However, may I suggest that Documentation/SubmittingPatches could do with a little rewording in this respect?
> Do not forget to add trailers such as "Acked-by:", "Reviewed-by:" and > "Tested-by:" lines as necessary to credit people who helped your > patch.
"Helped-by:" isn't even mentioned.
Show 5 quoted lines
> > +static void init_curl_proxy_auth(CURL *result)
> > +{
> > + env_override(&http_proxy_authmethod, "GIT_HTTP_PROXY_AUTHMETHOD");
>
> Shouldn't this also be part of the #if/#endif?The idea here was to have as little code as possible within the #if/#endif, as a matter of principle. It may be a little construed in this case, but supposing there's some subtle bug with env_override, or a future change introduces one, having it occur only for certain CURL versions would tend to make it harder to track down.
> and this code would be: > > if (remote) > var_override(&http_proxy_authmethod, remote->http_proxy_authmethod);
Good catch.
Cheers, Knut
-- Vorstandsvorsitzender/Chairman of the board of management: Gerd-Lothar Leonhart Vorstand/Board of Management: Dr. Bernd Finkbeiner, Dr. Arno Steitz Vorsitzender des Aufsichtsrats/ Chairman of the Supervisory Board: Philippe Miltin Sitz/Registered Office: Tuebingen Registergericht/Registration Court: Stuttgart Registernummer/Commercial Register No.: HRB 382196