On Sat, Feb 24, 2018 at 09:36:03PM +0700, Duy Nguyen wrote: > On Sat, Feb 24, 2018 at 10:34 AM, Nguyễn Thái Ngọc Duy > <pclo...@gmail.com> wrote: > > @@ -3995,6 +3995,18 @@ static void run_diff(struct diff_filepair *p, struct > > diff_options *o) > > return; > > } > > > > + /* > > + * NEEDSWORK: When running in no-index mode (and no repo is > > + * found, thus no hash algo conifugred), fall back to SHA-1 > > + * hashing (which is used by diff_fill_oid_info below) to > > + * avoid regression in diff output. > > + * > > + * In future, perhaps we can allow the user to specify their > > + * hash algorithm from command line in this mode. > > + */ > > + if (o->flags.no_index && !the_hash_algo) > > + the_hash_algo = &hash_algos[GIT_HASH_SHA1]; > > Brian, are we supposed to use the_hash_algo this way (i.e. as a > writable var)? Or should I stick to something like > > repo_set_hash_algo(the_repository, GIT_HASH_SHA1); > > which allows us to notify other parts inside struct repository about > the hash algorithm change, if we ever need to?
I would definitely recommend using the function. As you pointed out, it makes our code future-proof against needing to more work when setting the value. > If the_hash_algo is supposed to be read-only, maybe I should convert > that macro to an inline function to prevent people from accidentally > reassigning it? You could if you want to, although I don't really see a need to, since we can just catch it in review. If you wanted to, I'd make it an inline function for performance reasons. -- brian m. carlson / brian with sandals: Houston, Texas, US https://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: https://keybase.io/bk2204
signature.asc
Description: PGP signature
