On 8/8/2017 9:22 PM, Jonathan Tan wrote:
Here is the complete patch set. I have only moved the exported functions
that operate with packfiles and their static helpers - for example,
static functions like freshen_packed_object() that are used only by
non-pack-specific functions are not moved.

In the end, 3 functions needed to be made global. They are
find_pack_entry(), mark_bad_packed_object(), and has_packed_and_bad().

Of the 3, find_pack_entry() is probably legitimately promoted. But I
think that the latter two functions needing to be accessed from
sha1_file.c points to a design that could be improved - they are only
used when packed_object_info() detects corruption, and used for marking
as bad and printing messages to the user respectively, which
packed_object_info() should probably do itself. But I have not made this
change in this patch set.

(Other than the 3 functions above, there are some variables and
functions that are temporarily made global, but reduced back to static
when the wide scope is no longer needed.)


Nice to see the pack file functions being refactored out. I looked at the end result and it looked good to me.

Do you have the energy to do a similar refactoring for the remaining public functions residing in sha1_file.c? Perhaps a new sha1_file.h? It would be nice to get more things out of cache.h. :)

Reply via email to