From: Derrick Stolee Date: Thu, 02 Dec 2021 15:22:05 GMT Subject: Re: [PATCH 04/17] chunk-format.h: extract oid_version() Message-ID: <2d5456f6-5a4d-1600-83f3-2b6d3e1b270a@gmail.com> In-Reply-To: On 11/29/2021 5:25 PM, Taylor Blau wrote: > There are three definitions of an identical function which converts > `the_hash_algo` into either 1 (for SHA-1) or 2 (for SHA-256). There is a > copy of this function for writing both the commit-graph and > multi-pack-index file, and another inline definition used to write the > .rev header. > > Consolidate these into a single definition in chunk-format.h. It's not > clear that this is the best header to define this function in, but it > should do for now. Thanks for consolidating these! > (Worth noting, the .rev caller expects a 4-byte unsigned, but the other > two callers work with a single unsigned byte. The consolidated version > uses the latter type, and lets the compiler widen it when required). > > Another caller will be added in a subsequent patch. > chunk-format.c | 12 ++++++++++++ > chunk-format.h | 3 +++ > commit-graph.c | 18 +++--------------- > midx.c | 18 +++--------------- > pack-write.c | 15 ++------------- I notice that you don't use this in load_pack_mtimes_file(), in pack-mtimes.c but you could at this point. The code you do touch looks good. Thanks, -Stolee