On 2015-11-02 14:46, Junio C Hamano wrote:
> > Reviewed-by: Junio C Hamano <gits...@pobox.com>
> > Reviewed-by: Eric Sunshine <sunsh...@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.

> > +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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to