git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Stefan Beller <sbeller@google.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: "git@vger.kernel.org" <git@vger.kernel.org>,
	Duy Nguyen <pclouds@gmail.com>,
	Jens Lehmann <Jens.Lehmann@web.de>,
	Jacob Keller <jacob.keller@gmail.com>
Subject: Re: [PATCH 3/4] submodule--helper list: lose the extra prefix option
Date: Fri, 25 Mar 2016 09:49:56 -0700	[thread overview]
Message-ID: <CAGZ79kZ684W46df9zPQATr3zWKt+e1BhGY6DZ84psfXWH4tGNw@mail.gmail.com> (raw)
In-Reply-To: <xmqqmvpn5awo.fsf@gitster.mtv.corp.google.com>

On Thu, Mar 24, 2016 at 11:28 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
> So this change may not be wrong per-se, but if the lossage of prefix
> is the final goal (as opposed to an approach to gain other benefits,
> e.g. "now we do not have to use prefix, we can simplify these other
> things"), I do not know if it is worth it.
>

It is the final goal of this series. As motivation, see Jacobs comment above:

    I had wondered why we used --prefix before.

Also when the submodule helper got in, reviewers (Jonathan) were confused and
asked for clarification of the prefix term. So either document the prefix term
(and come up with a reason why we don't use the standard mechanism,
which as you outlined in the other mail, may be performance as we skip one
chdir, but that sounds like weak argument to me) or remove the confusing part
of having a prefix, which is not the standard prefix inside C.

The other reason you gave below is also convincing: By having it in the prefix,
the C code is more likely correct and future proof.

On rewriting the whole submodule command in C (probably reiterating):
It is not my endgoal to rewrite every submodule related part in C. It
would be nice
if it would happen eventually, but for now I only rewrite parts that I
need in C. (i.e.
paralllelisation is hard in shell, if not impossible using posix shell
with no additional
dependencies [xargs, gnu parallel], so I moved that part to C.
Certain parts need a performance boost? Ok I'll do it in C.

That said, we may have the shell/C architecture for a longer time than planned,
which makes it important to comment/document the confusing parts. Instead
I'd rather not have confusing parts, so I see benefit in having the
goal of this series
to remove the --prefix.

  parent reply	other threads:[~2016-03-25 16:50 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-24 23:34 [PATCHv2 0/4] submodule helper: cleanup prefix passing Stefan Beller
2016-03-24 23:34 ` [PATCH 1/4] submodule: fix recursive path printing from non root directory Stefan Beller
2016-03-24 23:38   ` Jacob Keller
2016-03-24 23:44     ` Stefan Beller
2016-03-25 16:43   ` Junio C Hamano
2016-03-25 16:54     ` Stefan Beller
2016-03-24 23:34 ` [PATCH 2/4] submodule: fix recursive execution " Stefan Beller
2016-03-24 23:41   ` Jacob Keller
2016-03-25 16:46   ` Junio C Hamano
2016-03-25 17:27     ` Stefan Beller
2016-03-24 23:34 ` [PATCH 3/4] submodule--helper list: lose the extra prefix option Stefan Beller
2016-03-24 23:44   ` Jacob Keller
2016-03-25  6:28   ` Junio C Hamano
2016-03-25 16:02     ` Junio C Hamano
2016-03-25 17:25       ` Junio C Hamano
2016-03-25 17:31         ` Stefan Beller
2016-03-25 16:49     ` Stefan Beller [this message]
2016-03-25 17:01       ` Junio C Hamano
2016-03-25 17:05         ` Stefan Beller
2016-03-25 18:32           ` Junio C Hamano
2016-03-24 23:34 ` [PATCH 4/4] submodule: add more tests for recursive submodule behavior Stefan Beller
2016-03-25  0:25   ` Eric Sunshine
2016-03-25  0:33     ` Stefan Beller

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=CAGZ79kZ684W46df9zPQATr3zWKt+e1BhGY6DZ84psfXWH4tGNw@mail.gmail.com \
    --to=sbeller@google.com \
    --cc=Jens.Lehmann@web.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jacob.keller@gmail.com \
    --cc=pclouds@gmail.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 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).