shejialuo <shejialuo@xxxxxxxxx> writes: > On Fri, Nov 15, 2024 at 02:42:19PM +0100, Karthik Nayak wrote: > > [snip] > >> diff --git a/midx.h b/midx.h >> index 78efa28d35371795fa33c68660278182debb60ab..7620820d4d0272926af9e4eeb68bfb73404c7ec2 100644 >> --- a/midx.h >> +++ b/midx.h >> @@ -7,6 +7,7 @@ struct object_id; >> struct pack_entry; >> struct repository; >> struct bitmapped_pack; >> +struct git_hash_algo; >> >> #define MIDX_SIGNATURE 0x4d494458 /* "MIDX" */ >> #define MIDX_VERSION 1 >> @@ -89,8 +90,10 @@ struct multi_pack_index { >> #define MIDX_EXT_MIDX "midx" >> >> const unsigned char *get_midx_checksum(struct multi_pack_index *m); >> -void get_midx_filename(struct strbuf *out, const char *object_dir); >> -void get_midx_filename_ext(struct strbuf *out, const char *object_dir, >> +void get_midx_filename(const struct git_hash_algo *hash_algo, >> + struct strbuf *out, const char *object_dir); >> +void get_midx_filename_ext(const struct git_hash_algo *hash_algo, >> + struct strbuf *out, const char *object_dir, >> const unsigned char *hash, const char *ext); > > I don't think it's a good idea to put "hash_algo" in the first argument, > we should put it at the last to align with the code style where we use > "git_hash_algo". > Could you elaborate on why you think it is not a good idea? I've mostly done this to stay consistent, because I see `struct repository *repo` being passed as the first variable in our code base. Roughly: $ grep -Iir "struct repository \*r" --include=\*.h | wc -l 524 $ grep -Iir "(struct repository \*r" --include=\*.h | wc -l 327 Since `hash_algo` is similar, I thought it would be nicer to be consistent. >> void get_midx_chain_dirname(struct strbuf *buf, const char *object_dir); >> void get_midx_chain_filename(struct strbuf *buf, const char *object_dir);
Attachment:
signature.asc
Description: PGP signature