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

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 11, 2025, 17:57 UTC
Message-ID: <xmqqikgl5nj3.fsf@gitster.g>
In-Reply-To: <98592a42-71de-d86e-a727-32115615a82d@mailbox.tu-dresden.de>

Thomas Uhle <thomas.uhle@mailbox.tu-dresden.de> writes:

Show 11 quoted lines

> On Fri, 10 Oct 2025, Junio C Hamano wrote:
>
>> Thomas Uhle <thomas.uhle@mailbox.tu-dresden.de> 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.