On Sun, Jul 22, 2018 at 10:50 AM, Duy Nguyen <pclouds@xxxxxxxxx> wrote:
> On Sun, Jul 22, 2018 at 7:51 AM Christian Couder
> <christian.couder@xxxxxxxxx> wrote:
>> +pack.island::
>> + A regular expression configuring a set of delta islands. See
>> + "DELTA ISLANDS" in linkgit:git-pack-objects[1] for details.
>> +
>
> That section is not added until 3/5 though.
Yeah, so I guess it is better to move this hunk to 3/5 and keep
pack.island undocumented until the delta islands code is actually used
by pack-objects.
>> diff --git a/delta-islands.c b/delta-islands.c
>> new file mode 100644
>> index 0000000000..645fe966c5
>> --- /dev/null
>> +++ b/delta-islands.c
>> @@ -0,0 +1,490 @@
>> +#include "builtin.h"
>
> A bit weird that builtin.h would be needed...
Yeah, I will get rid of that include in the next iteration.
>> + if (progress)
>> + progress_state = start_progress("Propagating island marks", nr);
>
> _() (same comment for other strings too)
Ok, the strings will be marked for translation in the next iteration.
>> diff --git a/pack-objects.h b/pack-objects.h
>> index edf74dabdd..8eecd67991 100644
>> --- a/pack-objects.h
>> +++ b/pack-objects.h
>> @@ -100,6 +100,10 @@ struct object_entry {
>> unsigned type_:TYPE_BITS;
>> unsigned no_try_delta:1;
>> unsigned in_pack_type:TYPE_BITS; /* could be delta */
>> +
>> + unsigned int tree_depth; /* should be repositioned for packing? */
>> + unsigned char layer;
>> +
>
> This looks very much like an optional feature. To avoid increasing
> pack-objects memory usage for common case, please move this to struct
> packing_data.
Ok, I will take a look at that.
Thanks for the review,
Christian.
