On Mon, Sep 23, 2013 at 11:49:01AM -0700, Brandon Casey wrote:

> Brandon Casey (16):
>   contrib/git-credential-gnome-keyring.c: remove unnecessary
>     pre-declarations
> [...]

Thanks, I think this is a good change. There were patches[1] from
Philipp about a year ago that went in the opposite direction, factoring
out common credential functions to be used by several helpers. We ended
up not merging them, because porting the wincred helper to the generic
implementation introduced complications.

And that was part of the reason for splitting out the helpers in the
first place: to let them worry about their own individual
system-specific quirks (and take advantage of system-specific helpers).
IOW, to make the gnome helper more gnome-y than git-y. And your patches
go in that direction, which I think is sane.

[1] http://thread.gmane.org/gmane.comp.version-control.git/204154/focus=204157

>   contrib/git-credential-gnome-keyring.c: report failure to store
>     password

Your subject lines are rather on the long side. I wonder if they would
be more readable as just:

  gnome-keyring: report failure to store password

and so forth. Anyone familiar with git.git knows that the "contrib"
and "git-credential-" bits are implied by "gnome-keyring". But it's not
a huge deal either way.

> Inserts a patch to fix the style issues for block statements.
> i.e. use "if ()" instead of "if()"

There are a ton of other style issues in the existing code (lack of
whitespace around operators and function arguments, and "char* foo"
instead of "char *foo" were the two I noticed). As a separate contrib/
project, I'm not that concerned, and I would say it is up to whoever is
nominally in charge of maintaining that contrib directory (Philipp or
John, in this case).

To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to