Felipe Contreras wrote: > On Tue, Feb 14, 2012 at 10:34 PM, Jonathan Nieder <jrnieder@xxxxxxxxx> wrote: >> To summarize the previous discussion[1]: some people had comments, and >> you seem to have found value in exactly none of them. OK. CC-ing >> Peff, since he at least probably has looked over this code before. > > Just because you have comments doesn't mean I *must* address them. We > have a difference of opinion, nothing wrong with that. I said "OK", didn't I? [...] >> In fact it seems to be intended to test the case addressed by f026358e >> (name changing, email not) in various mailmap callers: "git shortlog -e", >> "git log --pretty", "git blame". > > No. As the summary says, it's intended to add a simple name > translation test, which is missing from all the tests that spawn from > the repository generated in 'Shortlog output (complex mapping)' test. > This is the most minimal patch that can be generated if you add a > commit to this repository, and any further tests that are related to > it would look the same. > > As Junio pointed out what is missing from the explanation is that this > simple name translation test is targeted toward the 'git blame' > commands, because such translation is not tested for them currently. Um. So this has nothing to do with f026358e at all? Mentioning that commit and that this test does not pass with an older codebase is not useful to the humans that will look over this test later? Adding explanation and rearranging things so people encountering this later have to spend _less_ time to understand it is something I consider useful. It means people are less likely to randomly break things. I don't actually understand where the difference of opinion comes from here. -- 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