From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Erik Faye-Lund <kusmabite@gmail.com>,
"Philipp A. Hartmann" <pah@qo.cx>,
git@vger.kernel.org, John Szakmeister <john@szakmeister.net>
Subject: Re: [PATCH] contrib: GnomeKeyring support + generic helper implementation
Date: Fri, 24 Aug 2012 17:33:42 -0400 [thread overview]
Message-ID: <20120824213342.GB16285@sigill.intra.peff.net> (raw)
In-Reply-To: <7vfw7cyx4n.fsf@alter.siamese.dyndns.org>
On Fri, Aug 24, 2012 at 11:15:36AM -0700, Junio C Hamano wrote:
> > 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
next prev parent reply other threads:[~2012-08-24 21:34 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-08-23 16:57 [PATCH] contrib: GnomeKeyring support + generic helper implementation Philipp A. Hartmann
2012-08-23 16:57 ` [PATCH 1/4] contrib: add credential helper for GnomeKeyring Philipp A. Hartmann
2012-08-23 16:57 ` [PATCH 2/4] contrib: add generic credential helper Philipp A. Hartmann
2012-08-23 16:57 ` [PATCH 3/4] gnome-keyring: port to generic helper implementation Philipp A. Hartmann
2012-08-23 16:57 ` [PATCH 4/4] osxkeychain: port to generic credential " Philipp A. Hartmann
2012-08-24 18:15 ` [PATCH] contrib: GnomeKeyring support + generic " Junio C Hamano
2012-08-24 21:33 ` Jeff King [this message]
2012-08-24 21:46 ` Junio C Hamano
2012-08-26 17:46 ` Junio C Hamano
2012-08-26 18:16 ` Philipp A. Hartmann
2012-08-26 22:04 ` [PATCH 5/4] wincred: port to generic credential helper (UNTESTED) Philipp A. Hartmann
2012-08-26 22:45 ` [PATCH 5/4 v2] " Philipp A. Hartmann
2012-08-30 18:27 ` [PATCH 5/4] " Erik Faye-Lund
2012-08-30 20:11 ` Junio C Hamano
2012-08-31 15:44 ` Erik Faye-Lund
2012-09-01 12:42 ` [PATCH 5/4 v3] wincred: port to generic credential helper Philipp A. Hartmann
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=20120824213342.GB16285@sigill.intra.peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=john@szakmeister.net \
--cc=kusmabite@gmail.com \
--cc=pah@qo.cx \
/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).