Miklos Vajna wrote: > I guess that is a should-not-happen category. parse_args() calls > setup_revisions(), and that will already die() if the argument is not a > valid object at all. Then why do you have an if() guarding the code? In my opinion, you should have an else-clause that die()s with an appropriate message. > Yes. If you would want, I could of course add test cases for two other > cases when we already errored out and now the error message is just > changed, but I don't think duplicating the error message strings from > the code to the testsuite is really wanted. :-) Nope, I'd never suggest that: this is fine. What I meant is: you should clarify that you're fixing a bug and adding a test to guard it, in the commit message. -- 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