2009/11/3 Nanako Shiraishi <nanako3@xxxxxxxxxxx>: > If you are sending an update to a previous patch (I am comparing > this patch with the "show by example" patch Junio sent on 11/02 > http://article.gmane.org/gmane.comp.version-control.git/131893), > it is a common courtesy to summarize what you changed relative > to the base version after the three dashes line, so that people > will know which part can be skipped while reviewing your patch. I got your point. I will try to improve that. I have been talking to Junio during the weekend and with a lot of emails sent to each other. It happens that when he sent gmane/131893 (I had to find out what that code meant because I was using [marc.info]! :-) ) I had already sent another patch with the suggestions he made in a previous email. So his message was late. While I was waiting for his acknowledgment I started thinking he could be lost on those e-mails so I sent the one you are replying to make it the last on the queue. > I have to say that the test script is much worse than what > gmane/131893 had. > > The old test made sure that -C copied the message, with or > without the --mine option. But this test only checks the > author line (and it doesn't even make sure that "^author" > matches only in the header). The messages are unchecked, > and it will let a bug when someone breaks --reset-author > logic in the future in such a way that it corrupts the > message by mistake go unnoticed. I think you misunderstood something here: * On his patch (which he sent before seeing mine), while testing -C, on first check, he is checking author_header only. * While testing -C on mine I compare both messages without "parent", "tree" and "committer". Whole! After check one I did concentrate only on author data. But on mine I separate the tests between timestamp and author (name and email), making sure the author was set to the actual committer and that the timestamp was behaving as expected. I am not saying mine is better. What I am saying is that I expected him to notice and change/improve THIS patch. Not the other. The new option only touches on getting new author or copying the original so that is why I made the first check in whole and the others only by author. If people think that this operation is so uncertain, then everything should be compared: parent, author and message on all tests. It is not a problem for me adding more code to the test even if I consider it unnecessary. I am doing this only to give a pay-back for all the good service this free software gave to me so I am very patient to all demands. I will be letting this effort go only at the real end. No matter how long it takes. > Also the old test was much more readable because it used > shell functions to avoid repeating cat-file and pipe to sed > script. It also tagged the initial commit, which had a nice > effect that a failure in any intermediate test will not change > which earlier commit is reused (eg. yours say "-C HEAD^" but > old test said "-C Initial"). I am so used to scripts that I really haven't thought it difficult to read but I can do some cosmetic too if it is important. As I said early, I was waiting for Junio's jugdement over my later patch. > It looks silly to create an "Initial Commit" in the middle > of history, too (^_^). This is something more laborious but which I thought was important to let murphy's law act on a real case. We never do an amend on an initial commit so I only did the tests on a later one. > I think it is much better to replace "--mine" in gmane/131893 > with "--reset-author" and make no other change to the test. Let's see Junio's opinion... We have changed names a lot since start. ;-) > -- > Nanako Shiraishi > http://ivory.ap.teacup.com/nanako3/ > > Thank you very much for all your comments. I really appreciate about being noticed by you people. It is nice for a newcomer! -- 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