From: Junio C Hamano Date: Fri, 16 Apr 2010 20:23:30 GMT Subject: Re: [PATCH] Make rev_compare_tree less confusing. Message-ID: <7v1vefcfjx.fsf@alter.siamese.dyndns.org> In-Reply-To: <201004161131.06880.trast@student.ethz.ch> Thomas Rast writes: > Interestingly enough, this call survived with slight changes here and > there from all the way back in cf48454 (Teach git-rev-list to follow > just a specified set of files, 2005-10-20), where it was added in > rev-list.c. Even back then diff_tree() would always return 0. The idea always have been that diff_tree() may some day start returning non-fatal errors (otherwise it calls die() so that the caller does not even have a chance to worry about the return value), and the particular codepath the patch touches would treat such "one of the trees is unreadble so we cannot say if/how different they are" case as "there may be a change worth showing (even if the actual change couldn't be shown)". So I'm a bit reluctant to accept this patch under discussion. It doesn't change the behaviour, it doesn't make the _interface_ any simpler, and it removes the documentation value of what _should_ happen if diff_tree() were to be updated in such a way. It is entirely a different matter if the patch were to change the function signature of diff_tree() to return "void" and adjust all the callers involved. That will make the _interface_ simpler without changing the behaviour, and it makes it absolutely clear that we would _never_ enhance git to say "some necessary data for comparison is missing---we report error and allow the caller to err on the safe side and say 'there could be difference but the details we cannot say'".