On Thu, Mar 03, 2022 at 05:30:44PM +0100, Ævar Arnfjörð Bjarmason wrote:
>
> On Wed, Mar 02 2022, Taylor Blau wrote:
>
> > 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.
> > [...]
> > +
> > +uint8_t oid_version(const struct git_hash_algo *algop)
> > +{
> > + switch (hash_algo_by_ptr(algop)) {
> > + case GIT_HASH_SHA1:
> > + return 1;
> > + case GIT_HASH_SHA256:
> > + return 2;
>
> Not a new issue, but I wonder why these don't return hash_algo_by_ptr
> aka GIT_HASH_WHATEVER here. I.e. this is the same as this more
> straightforward & obvious code that avoids re-hardcoding the magic
> constants:
Hmm. Certainly the value returned by hash_algo_by_ptr() works for SHA-1
and SHA-256, but writes may want to use a different value for future
hashes. Not that this couldn't be changed then, but my feeling is that
the existing code is clearer since it avoids the reader having to jump
to hash_algo_by_ptr()'s implementation to figure out what it returns.
Thanks,
Taylor
