Re: [PATCH v2 00/25] Move exported packfile funcs to its own file
- From
Ben Peart <peartben@gmail.com>
- Date
- Aug 11, 2017, 19:41 UTC
- Message-ID
- <588e6770-58f8-0320-014b-0ce195d498ac@gmail.com>
- In-Reply-To
- <cover.1502241234.git.jonathantanmy@google.com>
On 8/8/2017 9:22 PM, Jonathan Tan wrote:
Show 20 quoted lines
> 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. :)