Re: [JGIT PATCH 1/2] add support for core.logAllRefUpdates configuration parameter

From: Robin Rosenberg <robin.rosenberg@dewire.com>
Date: Sep 25, 2009, 22:45 UTC
Message-ID: <200909260045.21171.robin.rosenberg@dewire.com>
In-Reply-To: <D35B4A582834DC418CCF9AF41AB69B70016953F62F@DEWDFECCR04.wdf.sap.corp>

onsdag 23 september 2009 18:42:29 skrev "Halstrick, Christian" <christian.halstrick@sap.com>:

Show 8 quoted lines

> From: Christian Halstrick <christian.halstrick@sap.com>
> 
> JGit should understand configuration parameter logAllRefUpdates and should
> only log updates of refs when
>   either the log file for this ref is already present
>   or this configuration parameter is set to true
> Before this commit JGit was always writing logs, regardless of this
> parameter or existence of logfiles.

A few minor things: Format the comment nicely and space betwee paragraphs. Start the comment with a capital letter A cover letter for multiple patches is nice, though in this case you could actually just squash there patches into one patch.

and one major. RefUpdateTest broke.

Probably because of this.

> +		logAllRefUpdates = rc.getBoolean("core", "logAllRefUpdates", false);

According to the git-config man page logAllRefUpdates is true by default for non-bare repos and false for bare repos.

I had some trouble applying these patches as git-am did not like them. Apparently git-am doesn't like base64. Could you set your mailer to use a more plain-ish format or use git-send-email.

-- robin