Re: [PATCH] contrib: GnomeKeyring support + generic helper implementation

From: Jeff King <peff@peff.net>
Date: Aug 24, 2012, 21:33 UTC
Message-ID: <20120824213342.GB16285@sigill.intra.peff.net>
In-Reply-To: <7vfw7cyx4n.fsf@alter.siamese.dyndns.org>

On Fri, Aug 24, 2012 at 11:15:36AM -0700, Junio C Hamano wrote:

Show 12 quoted lines

> >   The third and fourth patches port the existing helpers to this generic
> > implementation.
> >
> It struck me somewhat odd to see a new one added as the first step
> in the series, and then "the generic", the third patch only to lose
> code from the first one, and then use the same code reduction of
> existing one with the last one in the series.  A natural progression
> would have been the introduction of generic infrastructure
> (including the tiny bit this series had to add as part of 4/4) as
> the first patch, update existing OSX one to it as the second, and
> then build a new Gnome one on the infrastructure as the third and
> final patch; that way we have to review less code, too ;-).

I think this is partially because I talked with Phillipp off-list and was somewhat unsure how much we wanted to factor out of the helpers. My initial thought was that the protocol would be sufficiently simple that helpers would just be stand-alone and not need to share code with each other. Then the generic bits would not have to worry about being cross-platform compatible.

However, the shared bits are simple enough that maybe that is not a concern. An interesting test would be to add a 5/4 porting Erik's win32 credential helper, since that is the platform least like our other ones.

So I am OK with this series, but I am also OK with leaving it at patch 1, and just keeping the implementations separate.

-Peff