On Sat, Apr 20, 2013 at 03:38:53PM +0200, Michael J Gruber wrote: > > Wait, this does the opposite of the last patch. If we do want to do > > this, shouldn't the last one have been an "expect_failure"? > > The last patch just documents the status quo, which is not a bug per se. > Therefore, no failure, but change in the definition of "success". IMHO, the series is easier to review if you it does not go back and forth. If you have one patch that says "X is the right behavior", and then another patch that flips it to say "Y is the right behavior", the reviewer would read each in sequence and want to be convinced by your arguments for X and Y. But you probably cannot make a good argument for X if you are trying to end up at Y. :) So I'd much rather see the test introduced with the desired end behavior, marked as expect_failure, and the commit message contain an argument about why Y is a good thing (and squashing the tests in with the actual fix is often even better, because the fix itself would want to contain the same argument). Just my two cents as a reviewer. > My reasoning is twofold: > > - consistency between "git show commit" and "git show blob" I'm not sure I agree with this line of reasoning. "git show commit" is showing a diff, not the file contents; textconv has always been about munging the contents to produce a textual diff. It may be reasonable to extend its definition to "this is the preferred human view of this content, and that happens to be what you would want to produce a diff". But I do not think it is necessarily inconsistent not to apply it for the blob case. > - "git show" is a user facing command, and as such should produce output > consumable by humans; whereas "git cat-file" is plumbing and should > produce raw data unless told otherwise (-p, --textconv). That holds if the textconv is the only (or best) human-readable version of the file. And maybe that is reasonable. But is it possible that somebody uses "textconv" to produce a better diff of some already human-readable format? For example, imagine I define a textconv for XML files that normalizes the formatting to make diffs less noisy. When I am not looking at a diff, what is the best human-readable version? The original, or the normalized one? I'm not sure. Note that I'm somewhat playing devil's advocate here. For the cases where I have used textconv in the real world, I think I would probably prefer seeing the converted contents, and I am happy to call it user error if I use "git show HEAD:foo.jpg >bar.jpg" accidentally. But I also want to make sure we are not regressing somebody else unnecessarily. -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