All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Marco Trevisan via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, Marco Trevisan <mail@3v1n0.net>
Subject: Re: [PATCH] completion: use native ZSH array pattern matching
Date: Thu, 28 May 2020 09:14:59 -0700	[thread overview]
Message-ID: <xmqq8shc3ukc.fsf@gitster.c.googlers.com> (raw)
In-Reply-To: <pull.645.git.1590520398132.gitgitgadget@gmail.com> (Marco Trevisan via GitGitGadget's message of "Tue, 26 May 2020 19:13:17 +0000")

"Marco Trevisan via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= <mail@3v1n0.net>
>
> When clearing the builtin operations on re-sourcing in the ZSH case we
> can use the native ${parameters} associative array keys values to get
> the currently `__gitcomp_builtin_*` operations using pattern matching
> instead of using sed.

"We can" may be a statement of fact, but by itself it does not make
a convincing argument why we should have this change in our
codebase.  

Is this change about correctness?  Is it about performance?
Or something else?

If it is about correctness, "the current code fails in this way
given this input, but with the new code the breakage is gone" would
be a good justification to give in the proposed log message.  If it
is about performance, of course a measured performance numbers would
make an objective justification that is hard to argue with,
especially if you can convince people that the patch does not change
the behaviour in any negative way.  If it is about something else,
well, there would be an appropriate way to justify the change,
depending on what it is ;-)

Thanks.

      parent reply	other threads:[~2020-05-28 16:15 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-26 19:13 [PATCH] completion: use native ZSH array pattern matching Marco Trevisan via GitGitGadget
2020-05-26 23:58 ` brian m. carlson
2020-05-27  6:13   ` Carlo Marcelo Arenas Belón
2020-05-28  2:17     ` brian m. carlson
2020-05-28 15:54       ` Junio C Hamano
2020-05-28 22:57         ` brian m. carlson
2020-05-28 23:01           ` Junio C Hamano
2020-05-28 16:14 ` Junio C Hamano [this message]

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=xmqq8shc3ukc.fsf@gitster.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=mail@3v1n0.net \
    /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.