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 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