Re: [PATCH] Let "git svn" run "git gc --auto" occasionally

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 22, 2008, 01:52 UTC
Message-ID: <7vtzl6vd7v.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <20080122003911.GA16453@hand.yhbt.net>

Eric Wong <normalperson@yhbt.net> writes:

Show 25 quoted lines

>> >  sub init_vars {
>> > -	if (defined $_repack) {
>> > -		$_repack = 1000 if ($_repack <= 0);
>> > -		$_repack_nr = $_repack;
>> > -		$_repack_flags ||= '-d';
>> > -	}
>> > +	$_repack = 1000 unless (defined $_repack && $_repack > 0);
>> > +	$_repack_nr = $_repack;
>> > +	$_repack_flags ||= '-d';
>> >  }
>> >  
>> >  sub verify_remotes_sanity {
>> 
>> Thanks, but I think you need to do something about this part:
>> 
>> 2154:	if (defined $_repack && (--$_repack_nr == 0)) {
>> 
>> I'd say 
>> 
>> 	if ($_repack && (--$_repack_nr == 0)) {
>
> init_vars() is called unconditionally, and always defines $_repack.
> It could actually just be:
>
> 	if (--$_repack_nr == 0) {

But that means predecremented --$_repack_nr will count -1, -2, ... until it wraps around when the user said "--repack=0", meaning "never repack". Instead you made it "do not repack for a many many many rounds".

Which would be perfectly fine in practice but somehow feels a bit dirty to me.