Re: [Outreachy PATCH v2] credential-store: add lockTimeout config option

[Junio C Hamano <gitster@pobox.com>]

Okhuomon Ajayi <okhuomonajayi54@gmail.com> writes:

> Support credentialStore.lockTimeout as the preferred setting
> for credential lock retries, retaining lockTimeoutMS as a
> deprecated alias.
>
> Signed-off-by: Okhuomon Ajayi <okhuomonajayi54@gmail.com>

Hmph, does this change even make sense?

Sure, there are many variables whose name ends with "timeout".  But
are they all measured in milliseconds?  Or are some measured in
seconds and others measured in milliseconds?  If the latter, this
moves us in the wrong direction, no?  If the time unit is not
uniform across variables, it would be more sensible to give them
names with unit so the users would immediately know when they see

    cs.locktimeoutMilliSeconds = 500

that the wait will be subseconds.

On the other hand, if we are committed that all timeouts are
measured in milliseconds, such a unit suffix does not buy us much,
and better be dropped.

Anybody who reads your proposed log message above in the future will
ask these questions, and a log message that forces them to ask such
questions needs more work.  State why this patch decides to do this,
and do not say "somebody told me to make this change".  Your readers
want to hear why you think this change makes sense, and be
convinced.