Thread (1 message) 1 message, 1 author, 2016-06-15

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

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:48:17

Michael Lukashov [off-list ref] writes:
The following functions are duplicated:

  encode_header
what are the other duplicated ones ;-)?
Signed-off-by: Michael Lukashov <redacted>
---
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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help