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

Michael Lukashov <michael.lukashov@gmail.com> writes:

what are the other duplicated ones ;-)?

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.