On Thu, May 02, 2013 at 09:05:01AM +0200, Johannes Sixt wrote: > > I figured the lengthy description in the commit message would be > > sufficient, > > It's absolutely sufficient *if* one reads the commit message. In this > case, though it goes more like "this function should be trivial, and it is > -- up to this if statement; what the heck is it good for?" and the reader > is forced to dig the history. What I really didn't want to do was to rewrite the whole rationale again. I think something like: /* * Move the found object to the front of the collision chain; * this is a pure optimization that can make future lookups cheaper, * assuming there is some time-locality to which objects are looked * up. */ would be enough, and interested parties can go to the commit history (I tend to go there pretty rapidly if there is a chunk of code that sticks out, but I guess not everybody does). > BTW, do you notice that the function is now modifying an object (the hash > table) even though this is rather unexpected from a "lookup" function? I think this is fine. The function is conceptually constant from the outside; callers don't even know about the hash table. They just know that there is some mapping. It's similar to the way that lookup_commit will lazily allocate the "struct commit". The callers do not care whether it exists already or not; they care that at the end of the function, they have a pointer to the commit. Everything else is an implementation detail. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html