git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Stefan Beller <sbeller@google.com>
Cc: Junio C Hamano <gitster@pobox.com>,
	"git@vger.kernel.org" <git@vger.kernel.org>,
	Jonathan Nieder <jrnieder@gmail.com>,
	Johannes Sixt <j6t@kdbg.org>, Heiko Voigt <hvoigt@hvoigt.net>,
	Jens Lehmann <jens.lehmann@web.de>
Subject: Re: [RFC PATCH 2/3] run-commands: add an async queue processor
Date: Fri, 21 Aug 2015 15:51:35 -0400	[thread overview]
Message-ID: <20150821195135.GA26720@sigill.intra.peff.net> (raw)
In-Reply-To: <CAGZ79kYM6m-Me=okbxMAY_NKDm40JwfgRutwTzYfT5=ez42=jg@mail.gmail.com>

On Fri, Aug 21, 2015 at 12:48:23PM -0700, Stefan Beller wrote:

> > Before even looking at the implementation, my first question would be
> > whether this pattern is applicable in several places in git (i.e., is it
> > worth the extra complexity of abstracting out in the first place). I
> > think there are a few task-queue patterns already in git; for example
> > the delta search in pack-objects. Is the interface given here sufficient
> > to convert pack-objects? Is the result nicer to read? Is it as
> > efficient?
> >
> > We do not need to convert all possible call-sites to the new abstracted
> > code at once. But I find that converting at least _one_ is a good litmus
> > test to confirm that a new interface is generally useful.
> 
> Ok, I'll head off to convert one place.

Thanks. By the way, reading over what I wrote, it sounds a little
harsher than I meant. It is not "if you do not convert an existing site,
we cannot accept a new interface, period". But I would like at least
some explanation as an alternative, like "I'm pretty sure we can convert
site X to this, but it is not a good time to do so now because of Y".
Where "Y" might be "we need to do this other refactoring work first", or
"it would be disruptive to other work in the area".

-Peff

  reply	other threads:[~2015-08-21 19:51 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-21  1:40 [PATCH 1/3] submodule: implement `module_clone` as a builtin helper Stefan Beller
2015-08-21  1:40 ` [RFC PATCH 2/3] run-commands: add an async queue processor Stefan Beller
2015-08-21 19:05   ` Junio C Hamano
2015-08-21 19:44     ` Jeff King
2015-08-21 19:48       ` Stefan Beller
2015-08-21 19:51         ` Jeff King [this message]
2015-08-21 20:12           ` Stefan Beller
2015-08-21 20:41       ` Junio C Hamano
2015-08-21 23:40       ` Stefan Beller
2015-08-24 21:22         ` Junio C Hamano
2015-08-21 19:45     ` Stefan Beller
2015-08-21 20:47       ` Junio C Hamano
2015-08-21 20:56         ` Stefan Beller
2015-08-21  1:40 ` [WIP/PATCH 3/3] submodule: helper to run foreach in parallel Stefan Beller
2015-08-21 19:23   ` Junio C Hamano
2015-08-21 20:21     ` 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=20150821195135.GA26720@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=hvoigt@hvoigt.net \
    --cc=j6t@kdbg.org \
    --cc=jens.lehmann@web.de \
    --cc=jrnieder@gmail.com \
    --cc=sbeller@google.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).