git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org, Jeff Hostetler <git@jeffhostetler.com>,
	Jeff King <peff@peff.net>
Subject: Re: [PATCH v5 1/9] Start to implement a built-in version of `git add --interactive`
Date: Wed, 13 Nov 2019 12:54:29 +0900	[thread overview]
Message-ID: <xmqqd0dwbfd6.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <nycvar.QRO.7.76.6.1911121459270.46@tvgsbejvaqbjf.bet> (Johannes Schindelin's message of "Tue, 12 Nov 2019 16:03:45 +0100 (CET)")

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> Regardless of the language the scripted version was written in, I
>> think the '--helper' approach is always the poorer choice between
>> the two [*1*].  It limits the modular decomposition to what suits the
>> original language, the impedance mismatch between the original and
>> target language forces us to unnatural style of inter module
>> communication, and the unnatural interface layer, which we know has
>> to be discarded at the end, must be written [*2*].
>>
>> So, I'd prefer to see "because this is a better way in the longer
>> term" over "because the --helper approach would not work".
>
> Hmm. I feel distinctly unheard.

The feeling is mutual ;-)

> It may appear compelling, conceptually, to shun the `--helper` approach,
> but the more important reality is that it is the only one that makes an
> incremental conversion possible at all.
>
> It took an entire month of 60-hour weeks to complete the conversion of
> `git add -i`/`git add -p` to C, and only at the very end was I able to
> run the test suite with `GIT_TEST_ADD_I_USE_BUILTIN=true` and see it
> pass.

Yeah, that is developer comfort, and of course it is nice to have
than not to have it.

But compared to the downside impact to the quality of end result
that is inherent to the '--helper' approach, I'd prioritize the
quality of the end result over developer comfort.

> It is indeed _in spite of_ the success of the `--helper` approach that
> we cannot use it here.

As I do not see those past '--helper' ones necessarily successes, we
must agree to disagree here.

In any case, the log message needs to express why _you_ ended up
taking the non-helper approach.  Even though it is far less
relevant, compared to that, what other approach you instead wanted
to take, I do not veto you from having your own opinion.

>> [Footnote]
>>
>> *1* In only one case I would recommend using "--helper" approach,
>>     though.  When you are not expecting the developer to be able to
>>     come up with a better split of the program into modules than how
>>     the scripted version is, and you want to ensure that the
>>     developer have something to show when they faild to complete the
>>     project after N weeks.  You are a more experienced developer
>>     than an average GSoC student, and there is no pencils-down time,
>>     so the exception would not apply.
>>
>> *2* In "git submodule" for example it was quite natural for the
>>     module that gives a list of submodules with its traits the
>>     program cares about to be written as a shell function that
>>     writes the data to its standard output.  And consuming modules
>>     sit at the downstream of a pipe, accepting its output.  When you
>>     are writing these modules both in C, you wouldn't connect them
>>     with pipe to carry the list of submodules, but a piecemeal
>>     conversion using the "--helper" approach meant that there always
>>     remained _some_ consumer that wants to read from the pipe, so
>>     long after the module lister was rewritten in C, it still needed
>>     to support a mode where it sends its output to the pipe, instead
>>     of just passing an array of structures.
>>

  reply	other threads:[~2019-11-13  3:54 UTC|newest]

Thread overview: 124+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-10 17:37 [PATCH 00/11] git add -i: add a rudimentary version in C (supporting only status and help so far) Johannes Schindelin via GitGitGadget
2019-04-10 17:37 ` [PATCH 01/11] Start to implement a built-in version of `git add --interactive` Johannes Schindelin via GitGitGadget
2019-04-18 14:31   ` Jeff Hostetler
2019-04-18 16:06     ` Jeff King
2019-04-30 23:40       ` Johannes Schindelin
2019-05-01  2:21         ` Jeff King
2019-05-13 11:14           ` Johannes Schindelin
2019-04-10 17:37 ` [PATCH 02/11] diff: export diffstat interface Daniel Ferreira via GitGitGadget
2019-04-10 17:37 ` [PATCH 03/11] built-in add -i: implement the `status` command Daniel Ferreira via GitGitGadget
2019-04-10 17:37 ` [PATCH 04/11] built-in add -i: refresh the index before running `status` Johannes Schindelin via GitGitGadget
2019-04-10 17:37 ` [PATCH 05/11] built-in add -i: color the header in the `status` command Johannes Schindelin via GitGitGadget
2019-04-10 17:37 ` [PATCH 07/11] Add a function to determine unique prefixes for a list of strings Slavica Djukic via GitGitGadget
2019-04-18 17:57   ` Jeff Hostetler
2019-05-13 12:48     ` Johannes Schindelin
2019-04-10 17:37 ` [PATCH 06/11] built-in add -i: implement the main loop Johannes Schindelin via GitGitGadget
2019-04-18 16:49   ` Jeff Hostetler
2019-05-13 12:04     ` Johannes Schindelin
2019-04-10 17:37 ` [PATCH 08/11] built-in add -i: show unique prefixes of the commands Slavica Djukic via GitGitGadget
2019-04-10 17:37 ` [PATCH 09/11] built-in add -i: support `?` (prompt help) Johannes Schindelin via GitGitGadget
2019-04-10 17:37 ` [PATCH 10/11] built-in add -i: use color in the main loop Slavica Djukic via GitGitGadget
2019-04-10 17:37 ` [PATCH 11/11] built-in add -i: implement the `help` command Johannes Schindelin via GitGitGadget
2019-05-13 17:27 ` [PATCH v2 00/11] git add -i: add a rudimentary version in C (supporting only status and help so far) Johannes Schindelin via GitGitGadget
2019-05-13 17:27   ` [PATCH v2 01/11] Start to implement a built-in version of `git add --interactive` Johannes Schindelin via GitGitGadget
2019-05-13 17:27   ` [PATCH v2 02/11] diff: export diffstat interface Daniel Ferreira via GitGitGadget
2019-05-13 17:27   ` [PATCH v2 03/11] built-in add -i: implement the `status` command Daniel Ferreira via GitGitGadget
2019-05-13 17:27   ` [PATCH v2 04/11] built-in add -i: refresh the index before running `status` Johannes Schindelin via GitGitGadget
2019-05-13 17:27   ` [PATCH v2 05/11] built-in add -i: color the header in the `status` command Johannes Schindelin via GitGitGadget
2019-05-13 17:27   ` [PATCH v2 06/11] built-in add -i: implement the main loop Johannes Schindelin via GitGitGadget
2019-05-13 17:27   ` [PATCH v2 07/11] Add a function to determine unique prefixes for a list of strings Slavica Djukic via GitGitGadget
2019-05-13 17:27   ` [PATCH v2 08/11] built-in add -i: show unique prefixes of the commands Slavica Djukic via GitGitGadget
2019-05-13 17:28   ` [PATCH v2 09/11] built-in add -i: support `?` (prompt help) Johannes Schindelin via GitGitGadget
2019-05-13 17:28   ` [PATCH v2 10/11] built-in add -i: use color in the main loop Slavica Djukic via GitGitGadget
2019-05-13 17:28   ` [PATCH v2 11/11] built-in add -i: implement the `help` command Johannes Schindelin via GitGitGadget
2019-07-16 14:58   ` [PATCH v3 00/11] git add -i: add a rudimentary version in C (supporting only status and help so far) Johannes Schindelin via GitGitGadget
2019-07-16 14:58     ` [PATCH v3 01/11] Start to implement a built-in version of `git add --interactive` Johannes Schindelin via GitGitGadget
2019-07-31 17:52       ` Junio C Hamano
2019-08-26 21:26         ` Johannes Schindelin
2019-08-27 22:25           ` Junio C Hamano
2019-08-28 15:06             ` Johannes Schindelin
2019-08-28 15:37               ` Junio C Hamano
2019-07-16 14:58     ` [PATCH v3 03/11] built-in add -i: implement the `status` command Daniel Ferreira via GitGitGadget
2019-07-31 18:12       ` Junio C Hamano
2019-08-27 10:04         ` Johannes Schindelin
2019-07-16 14:58     ` [PATCH v3 02/11] diff: export diffstat interface Daniel Ferreira via GitGitGadget
2019-07-31 17:59       ` Junio C Hamano
2019-08-27  9:22         ` Johannes Schindelin
2019-07-16 14:58     ` [PATCH v3 04/11] built-in add -i: refresh the index before running `status` Johannes Schindelin via GitGitGadget
2019-07-16 14:58     ` [PATCH v3 05/11] built-in add -i: color the header in the `status` command Johannes Schindelin via GitGitGadget
2019-07-16 14:58     ` [PATCH v3 06/11] built-in add -i: implement the main loop Johannes Schindelin via GitGitGadget
2019-07-31 18:14       ` Junio C Hamano
2019-07-16 14:58     ` [PATCH v3 07/11] Add a function to determine unique prefixes for a list of strings Slavica Djukic via GitGitGadget
2019-07-31 18:39       ` Junio C Hamano
2019-08-24 12:38       ` SZEDER Gábor
2019-08-27 12:14         ` Johannes Schindelin
2019-08-28 16:30           ` SZEDER Gábor
2019-08-28 16:34             ` [PATCH] [PoC] A simpler find_unique_prefixes() implementation SZEDER Gábor
2019-08-30 20:12               ` Johannes Schindelin
2019-07-16 14:58     ` [PATCH v3 08/11] built-in add -i: show unique prefixes of the commands Slavica Djukic via GitGitGadget
2019-07-16 14:58     ` [PATCH v3 09/11] built-in add -i: support `?` (prompt help) Johannes Schindelin via GitGitGadget
2019-07-16 14:58     ` [PATCH v3 10/11] built-in add -i: use color in the main loop Slavica Djukic via GitGitGadget
2019-07-16 14:58     ` [PATCH v3 11/11] built-in add -i: implement the `help` command Johannes Schindelin via GitGitGadget
2019-08-02 21:04       ` Junio C Hamano
2019-08-02 22:26         ` Jeff King
2019-07-16 18:38     ` [PATCH v3 00/11] git add -i: add a rudimentary version in C (supporting only status and help so far) Johannes Schindelin
2019-08-02 21:06       ` Junio C Hamano
2019-08-27 12:57     ` [PATCH v4 " Johannes Schindelin via GitGitGadget
2019-08-27 12:57       ` [PATCH v4 01/11] Start to implement a built-in version of `git add --interactive` Johannes Schindelin via GitGitGadget
2019-08-27 12:57       ` [PATCH v4 02/11] diff: export diffstat interface Daniel Ferreira via GitGitGadget
2019-08-27 12:57       ` [PATCH v4 04/11] built-in add -i: refresh the index before running `status` Johannes Schindelin via GitGitGadget
2019-08-27 12:57       ` [PATCH v4 03/11] built-in add -i: implement the `status` command Daniel Ferreira via GitGitGadget
2019-08-27 12:57       ` [PATCH v4 05/11] built-in add -i: color the header in " Johannes Schindelin via GitGitGadget
2019-08-27 12:57       ` [PATCH v4 07/11] Add a function to determine unique prefixes for a list of strings Slavica Djukic via GitGitGadget
2019-08-27 12:57       ` [PATCH v4 06/11] built-in add -i: implement the main loop Johannes Schindelin via GitGitGadget
2019-08-27 12:57       ` [PATCH v4 08/11] built-in add -i: show unique prefixes of the commands Slavica Djukic via GitGitGadget
2019-08-27 12:58       ` [PATCH v4 09/11] built-in add -i: support `?` (prompt help) Johannes Schindelin via GitGitGadget
2019-08-27 12:58       ` [PATCH v4 11/11] built-in add -i: implement the `help` command Johannes Schindelin via GitGitGadget
2019-08-27 12:58       ` [PATCH v4 10/11] built-in add -i: use color in the main loop Slavica Djukic via GitGitGadget
2019-11-04 12:15       ` [PATCH v5 0/9] git add -i: add a rudimentary version in C (supporting only status and help so far) Johannes Schindelin via GitGitGadget
2019-11-04 12:15         ` [PATCH v5 1/9] Start to implement a built-in version of `git add --interactive` Johannes Schindelin via GitGitGadget
2019-11-08  4:49           ` Junio C Hamano
2019-11-09 11:06             ` Johannes Schindelin
2019-11-10  7:18               ` Junio C Hamano
2019-11-11  9:15                 ` Johannes Schindelin
2019-11-11 12:09                   ` Junio C Hamano
2019-11-12 15:03                     ` Johannes Schindelin
2019-11-13  3:54                       ` Junio C Hamano [this message]
2019-11-13 12:30                         ` Johannes Schindelin
2019-11-13 14:01                           ` Junio C Hamano
2019-11-04 12:15         ` [PATCH v5 2/9] diff: export diffstat interface Daniel Ferreira via GitGitGadget
2019-11-08  4:56           ` Junio C Hamano
2019-11-04 12:15         ` [PATCH v5 3/9] built-in add -i: implement the `status` command Daniel Ferreira via GitGitGadget
2019-11-08  5:01           ` Junio C Hamano
2019-11-04 12:15         ` [PATCH v5 4/9] built-in add -i: color the header in " Slavica Đukić via GitGitGadget
2019-11-04 12:15         ` [PATCH v5 5/9] built-in add -i: implement the main loop Johannes Schindelin via GitGitGadget
2019-11-08  5:17           ` Junio C Hamano
2019-11-09 11:21             ` Johannes Schindelin
2019-11-04 12:15         ` [PATCH v5 6/9] built-in add -i: show unique prefixes of the commands Johannes Schindelin via GitGitGadget
2019-11-04 12:15         ` [PATCH v5 7/9] built-in add -i: support `?` (prompt help) Johannes Schindelin via GitGitGadget
2019-11-04 12:15         ` [PATCH v5 8/9] built-in add -i: use color in the main loop Slavica Đukić via GitGitGadget
2019-11-04 12:15         ` [PATCH v5 9/9] built-in add -i: implement the `help` command Slavica Đukić via GitGitGadget
2019-11-13 12:40         ` [PATCH v6 0/9] git add -i: add a rudimentary version in C (supporting only status and help so far) Johannes Schindelin via GitGitGadget
2019-11-13 12:40           ` [PATCH v6 1/9] Start to implement a built-in version of `git add --interactive` Johannes Schindelin via GitGitGadget
2019-11-14  2:15             ` Junio C Hamano
2019-11-14 15:07               ` Johannes Schindelin
2019-11-15  4:35                 ` Junio C Hamano
2019-11-13 12:40           ` [PATCH v6 2/9] diff: export diffstat interface Daniel Ferreira via GitGitGadget
2019-11-13 12:40           ` [PATCH v6 3/9] built-in add -i: implement the `status` command Daniel Ferreira via GitGitGadget
2019-11-13 12:41           ` [PATCH v6 4/9] built-in add -i: color the header in " Slavica Đukić via GitGitGadget
2019-11-13 12:41           ` [PATCH v6 5/9] built-in add -i: implement the main loop Johannes Schindelin via GitGitGadget
2019-11-13 12:41           ` [PATCH v6 6/9] built-in add -i: show unique prefixes of the commands Johannes Schindelin via GitGitGadget
2019-11-13 12:41           ` [PATCH v6 7/9] built-in add -i: support `?` (prompt help) Johannes Schindelin via GitGitGadget
2019-11-13 12:41           ` [PATCH v6 8/9] built-in add -i: use color in the main loop Slavica Đukić via GitGitGadget
2019-11-13 12:41           ` [PATCH v6 9/9] built-in add -i: implement the `help` command Slavica Đukić via GitGitGadget
2019-11-13 12:46           ` [PATCH v6 0/9] git add -i: add a rudimentary version in C (supporting only status and help so far) Johannes Schindelin
2019-11-15 11:11           ` [PATCH v7 " Johannes Schindelin via GitGitGadget
2019-11-15 11:11             ` [PATCH v7 1/9] Start to implement a built-in version of `git add --interactive` Johannes Schindelin via GitGitGadget
2019-11-15 11:11             ` [PATCH v7 2/9] diff: export diffstat interface Daniel Ferreira via GitGitGadget
2019-11-15 11:11             ` [PATCH v7 3/9] built-in add -i: implement the `status` command Daniel Ferreira via GitGitGadget
2019-11-15 11:11             ` [PATCH v7 4/9] built-in add -i: color the header in " Slavica Đukić via GitGitGadget
2019-11-15 11:11             ` [PATCH v7 5/9] built-in add -i: implement the main loop Johannes Schindelin via GitGitGadget
2019-11-15 11:11             ` [PATCH v7 6/9] built-in add -i: show unique prefixes of the commands Johannes Schindelin via GitGitGadget
2019-11-15 11:11             ` [PATCH v7 7/9] built-in add -i: support `?` (prompt help) Johannes Schindelin via GitGitGadget
2019-11-15 11:11             ` [PATCH v7 8/9] built-in add -i: use color in the main loop Slavica Đukić via GitGitGadget
2019-11-15 11:11             ` [PATCH v7 9/9] built-in add -i: implement the `help` command Slavica Đukić via GitGitGadget

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=xmqqd0dwbfd6.fsf@gitster-ct.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@jeffhostetler.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=peff@peff.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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).