git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Thomas Uhle <thomas.uhle@mailbox.tu-dresden.de>
Cc: <git@vger.kernel.org>
Subject: Re: [PATCH] contrib/credential: Amend and harmonize Makefiles
Date: Sat, 11 Oct 2025 10:57:52 -0700	[thread overview]
Message-ID: <xmqqikgl5nj3.fsf@gitster.g> (raw)
In-Reply-To: <98592a42-71de-d86e-a727-32115615a82d@mailbox.tu-dresden.de> (Thomas Uhle's message of "Sat, 11 Oct 2025 14:45:50 +0200")

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

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

  reply	other threads:[~2025-10-11 17:57 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-10 17:30 [PATCH] contrib/credential: Amend and harmonize Makefiles Thomas Uhle
2025-10-10 19:45 ` Junio C Hamano
2025-10-10 21:03   ` Thomas Uhle
2025-10-10 21:25     ` Junio C Hamano
2025-10-11 12:45       ` Thomas Uhle
2025-10-11 17:57         ` Junio C Hamano [this message]
2025-10-11 19:18           ` Thomas Uhle
2025-10-20 18:20 ` [PATCH v2] " Thomas Uhle

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqikgl5nj3.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=thomas.uhle@mailbox.tu-dresden.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).