Subject: Re: [PATCH 00/11] Some fixes and bunch of object_id conversions

From: "brian m. carlson" <[email-masked]>
Date: Sat, 20 Jan 2018 20:58:30 +0000

On Thu, Jan 18, 2018 at 03:50:52PM +0100, Patryk Obara wrote:
> * brian m. carlson implemented vtable for hash algorithm selection and pushed
> the repository format front - thanks to him it's now quite easy to
> experimentally replace hashing functions and, e.g. do some performance testing.

Good, I'm glad this has been helpful.

> Patch 1 is not directly related to object_id conversions but helped with
> debugging t5540, which kept failing on master for me (spoiler: it was Fedora
> fault).  It helps with debugging of failing git-push over HTTP in case of
> internal server error, so I think it might be worthwhile.
> 
> Patch 2 is a small adjustment to .clang-format, which prevents unnecessary
> line breaks after function return type.

I have no strong opinions about these two patches, but didn't see
anything that looked problematic.  Better debugging is always nice.

> Patch 6 is a tiny fix in oidclr function.

I think this is a good direction to go in.

> All other patches are progressive conversions to struct object_id with some
> formatting fixes sprinkled in. These should be somewhat uncontroversial, I hope.

Overall, I like the direction this series is going.

When I've made changes to the sha1_file functions, I've traditionally
moved them away from using "sha1_file" to "object_file" to ensure that
we make it a bit more obvious that they handle object_id structs and
aren't limited to SHA-1.  For consistency, it might be nice to make that
change.

Other than that and the question I had about the formatting, I think the
series looks good.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204