Re: [PATCH v2] fast-import: implement unpack limit

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Eric Wong <normalperson@xxxxxxxx> writes:
> > +	argv_array_push(&unpack.args, "unpack-objects");
> > +	argv_array_push(&unpack.args, "-q");
> > +
> > +	return run_command(&unpack);
 
> Looks good.  I haven't thought if "-q" is appropriate or not though.

Oops, I think tying it to the existing --quiet option in PATCH v3
would be good.

> > @@ -972,6 +990,12 @@ static void end_packfile(void)
> >  		fixup_pack_header_footer(pack_data->pack_fd, pack_data->sha1,
> >  				    pack_data->pack_name, object_count,
> >  				    cur_pack_sha1, pack_size);
> > +
> > +		if (object_count <= unpack_limit) {
> > +			if (loosen_small_pack(pack_data) == 0)
> > +				goto discard_pack;
> > +		}
> 
> "if (!loosen_small_pack(pack_data))" would be more idiomatic, but
> the logic is very clear here.  We haven't created the idx, we skip
> the part that creates the idx and instead jump directly to the part
> that closes and unlinks it.

I was on the fence about "!" vs "== 0" vs something else, too;
and I get thrown off by things like "!strcmp" in C all the time.
I can change it to "if (!loosen_small_pack(pack_data))" in v3
(probably in a day or so in case there's further comments)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]