Re: [PATCH v2 3/4] Refactoring: move duplicated code from builtin-pack-objects.c and fast-import.c to object.c

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

 



Michael Lukashov <michael.lukashov@xxxxxxxxx> writes:

> The following functions are duplicated:
>
>   encode_header

what are the other duplicated ones ;-)?

> Signed-off-by: Michael Lukashov <michael.lukashov@xxxxxxxxx>
> ---

Two comments:

 - encode_header() was a perfectly good name for a static function in
   these two contexts, but when lifted into public namespace, it is not
   clear enough anymore.  It is not clear "header" in what context you are
   talking about.  At least it should be encode_in_pack_object_header();

 - Look at what are in object.[ch]; they are all about "object" layer,
   that sits one level higher in the abstraction on top of the raw object
   data layer (e.g. read_sha1_file() and friends).  This function belongs
   to a layer that is even lower level than the raw object data (i.e. one
   particular implementation of the raw object data representations among
   others).

   It looks very out of place.  I would say that cache.h and sha1_file.c
   would probably be a better place, if nobody else finds a better
   alternative.

Other than that, I agree with the patch, including its choice of types
involved.
--
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]