All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: John Cai <jcai@gitlab.com>
Cc: Erik Cervin Edin <erik@cervined.in>, git@vger.kernel.org
Subject: Re: bug: git name-rev --stdin --no-undefined on detached head
Date: Fri, 24 Dec 2021 16:35:52 -0800	[thread overview]
Message-ID: <xmqqsfuh1pxz.fsf@gitster.g> (raw)
In-Reply-To: <xmqqk0ft3i3g.fsf@gitster.g> (Junio C. Hamano's message of "Fri, 24 Dec 2021 11:42:27 -0800")

Junio C Hamano <gitster@pobox.com> writes:

> Two possible things we can do to improve are
>
>  * Fix the documentation; it is not for scripters but for annotating
>    text with object names.
>
>  * Possibly add --names-from-standard-input option that would behave
>    more like "we cannot afford to stuff all object names on the
>    command line, so we feed them one by one from the standard input"
>    mode the "--stdin" option of other subcommands use.
>
> I do not think the latter is so important, as it is perfectly OK to
> use xargs to split the large input into multiple invocations of
> name-rev.  This is unlike "pack-objects --stdin" where the command
> needs to see _all_ input in a single invocation.

This is primarily to teach newer developers how incompatible changes
are done in this project.  I am not suggesting that introducing such
an incompatible change and following through these steps to the end
is a good idea in this case.

Hypothetically, if name-rev were so important a command that the
xargs based workaround were unacceptable, what we would probably
do is to make the following changes over time.

 1. Introduce a new `--annotate-text` option, which is a synonym to
    the current `--stdin` option.  `--stdin` will work the same way
    as today, but using it will issue a warning() to the standard
    error to tell people to use the `--annotate-text` option
    instead.

 2. After a while, change `--stdin` to die() with a suggestion that
    people should instead pipe into xargs to invoke name-rev
    instead.

 3. After the above two steps, users and scripts of `--stdin` will
    go extinct.  Reintroduce `--stdin` but with a behaviour more in
    line with the `--stdin` option of other subcommands, i.e. take
    one arg per line from the standard input, and behave as if they
    came in the argv[] array.

The idea is to give users an early warning that is annoying enough
to encourage migration, ample time to adjust to the new world order,
and to ensure that there are no stragglers that gets hurt when the
name gets reused for an option with totally different behaviour.

It may not be a bad idea to do steps #1 and #2, though.  I do not
think xargs name-rev is bad enough to require going to the step #3,
but `--stdin` that exists and does a wrong thing is much worse than
`--stdin` that does not exist, and finishing upto step #2 is enough
to rectify that.

  parent reply	other threads:[~2021-12-25  0:35 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-22 10:05 bug: git name-rev --stdin --no-undefined on detached head Erik Cervin Edin
2021-12-23 18:39 ` John Cai
2021-12-24  6:09   ` John Cai
2021-12-24 11:44     ` Erik Cervin Edin
2021-12-24 19:42   ` Junio C Hamano
2021-12-24 20:09     ` John Cai
2021-12-25  0:35     ` Junio C Hamano [this message]
2021-12-31 17:16     ` Philip Oakley

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=xmqqsfuh1pxz.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=erik@cervined.in \
    --cc=git@vger.kernel.org \
    --cc=jcai@gitlab.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.