Subject: Re: [PATCH] contrib/credential: Amend and harmonize Makefiles

From: Junio C Hamano <[email-masked]>
Date: Sat, 11 Oct 2025 10:57:52 -0700

Thomas Uhle <[email-masked]> writes:

> On Fri, 10 Oct 2025, Junio C Hamano wrote:
>
>> Thomas Uhle <[email-masked]> writes:
>>
>> >> Content-Type: text/plain; format=flowed; charset="US-ASCII"
>>
>> Please make sure your MUA does not corrupt whitespaces by sending
>> your e-mails with "format=flowed"
>
> Shall I simply resend the patch unchanged without "format=flowed" or has 
> it to be a v2 patch then?

That was more to remind you before you actually need to send a
second version (or another topic).  Of course, sending an email to
yourself as practice to make sure it won't come as flowed text would
be a good idea, but straight resend is probably not needed.  Please
fetch from my 'seen' branch from any of the public mirrors, and
check what is queued as ac6152f0 (contrib/credential: Amend and
harmonize Makefiles, 2025-10-10) is what you expected me to have
without your mailer corrupting the patch contents.

> Should I also rename $(MAIN) to $(GIT_CREDENTIAL_HELPER)?

I do not think such a change would add any value.  

If the original did not use such an intermediate macro, adding to
use it may or may not have added value for "not having to repeat",
but it meant that now you have to repeat MAIN over and over, need to
still make sure you do not mistype it as MIAN, burdening the readers
to hold in their head what $(MAIN) exactly referred to while reading
the file.  Makefile language does not offer warnings when you refer
to an undefined macros, so using $(GIT_CREDENTIAL_HELPER) that is
more prone to mistyping than $(MAIN), while it makes it slightly
easier to readers to follow, would not protect you from typos.  Not
using a macro at all and saying "git-credential-helper" when you
mean it would have the same effect.

So it smells that viable choices are only three:

 * if the original did not use $(MAIN), leave them as-is and spell
   the values (like "git-credential-osxkeychain") out.

 * if the original did use $(MAIN), leave them as-is, without rename
   it to a longer and more typo-prone $(GIT_CREDENTIAL_HELPER).  Or

 * if the original did use $(MAIN), spell the values out instead.

I prefer to do "clean-up" patches and "functional" patches
separately, and introduction of the install target is the latter, so
perhaps leave all the changes to Makefile macro trick out of this
patch and concentrate only on the new "install" target?  And then do
"clean-up" using Makefile macro if you want, with merit of such a
change defended separately.

Thanks.