On Sat, Nov 03, 2007 at 11:47:44AM -0700, Junio C Hamano wrote: (...) > But in this case, as the variable "sp" is never used before it > is reassigned, I can easily say "drop the useless assignment to > sp there". ;-) You got me here ;) (...) > When prev is not NULL but points at a null_sha1 nobody writes > anything out. Is this intended? > > In fact, the calling site always passes prev which is > prev[] in cmd_tag() and cannot be non-NULL. Damn, I overlooked this, and since the test suite doesn't do anything on that, that got through. Indeed either the test can be removed, since write_tag_body does the is_null_sha1() test, or the is_null_sha1() test can be moved here. > Why is there "else" in the first place? Even if you start with > the previous tag's message, you are launching the editor for the > user to further edit it, and you would want to give some > instructions, wouldn't you? Well, it could be true if the text was more verbose than "Write a tag message". Anyways, as the test is now, the text is not going to appear. :( I'll fix this and will try to enhance the test suite to catch these problems. Mike - 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