On Tue, Feb 28, 2017 at 12:25:20PM -0800, Linus Torvalds wrote:
> On Tue, Feb 28, 2017 at 11:53 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> > Sorry, but at this point in your description, you completely lost
> > me. I thought "struct object_id" was what you call "hash_t" in the
> > above.
>
> So what happened was that I started out just encapsulating
>
> unsigned char sha1[20];
>
> as a
>
> hash_t hash;
>
> and that made sense in a lot of situations. I always thought that code that used
>
> struct object_id oid;
>
> is just too ugly to live, so I'm not actually all that big of a fan of
> the oid approach.
There was some discussion on the list about the best name to use, and
object_id seemed like the most popular decision. I don't care if we add
a typedef for it and prefer that typedef, but the existing code avoided
typedefs in favor of explicit struct definitions.
I'm certainly not opposed to having less to type, because “object_id” is
awkward to type, but I've generally tried to defer to existing uses in
the codebase and what list regulars are comfortable with.
The only problem with using hash_t is that it's then not obvious as we
transition (assuming we don't take an omnibus patch) what is converted
and what isn't.
> But the two approaches really are pretty much equivalent logically,
> even if they don't look the same.
Yeah, I think they are.
> So I wanted to unify things: "One type to bring them all and in the
> darkness bind them".
>
> So I just basically made this:
>
> typedef struct object_id {
> unsigned char hash[GIT_HASH_SIZE];
> } hash_t;
>
> to create one single data structure that doesn't make my eyes bleed.
> That "struct object_id" still exists, but I don't generally have to
> look at it when doing the conversion, and any current users "just
> work".
There is nothing that prevents us from doing a nice global
search-and-replace in the future if we think the status quo is bad.
That's something that could be automated with Coccinelle.
--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204
Attachment:
signature.asc
Description: PGP signature