git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Philipp A. Hartmann" <pah@qo.cx>
To: Junio C Hamano <gitster@pobox.com>
Cc: Erik Faye-Lund <kusmabite@gmail.com>, Jeff King <peff@peff.net>,
	git@vger.kernel.org, John Szakmeister <john@szakmeister.net>
Subject: Re: [PATCH] contrib: GnomeKeyring support + generic helper implementation
Date: Sun, 26 Aug 2012 20:16:44 +0200	[thread overview]
Message-ID: <503A680C.3090406@qo.cx> (raw)
In-Reply-To: <7vr4qtv94h.fsf@alter.siamese.dyndns.org>

n 26/08/12 19:46, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> 
>> Jeff King <peff@peff.net> writes:
>>
>>> 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.
>>
>> Very true.

In principle, I agree that with the current set of helper
implementations, the generic infrastructure may not yet pay off.

>>> So I am OK with this series, but I am also OK with leaving it at patch
>>> 1, and just keeping the implementations separate.
>>
>> Amen.
> 
> Just to make sure we do not leave loose ends, could somebody try to
> see if the new "generic helper" infrastructure is useful to shrink
> Erik's win32 credential helper implementation?

I'll try to give it a shot now, although I won't be able to test it.
I'll send the results as a single 5/4 addition to the previous series.

> If we see much code reduction and improved clarity, this refactoring
> may worth keeping.  Otherwise it may be sufficient to drop the later
> ones in the series.  Without knowing which, it is hard to decide.

If we decide that the generic implementation is useful, I'll then resend
the reordered series adding the generic helper first, then refactoring
the existing ones and adding the new one for GnomeKeyring last.

Thanks,
  Philipp

  reply	other threads:[~2012-08-26 18:25 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
2012-08-24 21:46     ` Junio C Hamano
2012-08-26 17:46       ` Junio C Hamano
2012-08-26 18:16         ` Philipp A. Hartmann [this message]
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=503A680C.3090406@qo.cx \
    --to=pah@qo.cx \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=john@szakmeister.net \
    --cc=kusmabite@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.
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).