Eric Wong <normalperson@xxxxxxxx> writes:
>> > 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.
-
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
