From: Ramkumar Ramachandra <artagnon@gmail.com>
Date: Thu, 11 Apr 2013 11:42:06 GMT
Subject: Re: [PATCH v2] cherry-pick: make sure all input objects are commits
Message-ID: <CALkWK0kb+2KZLvRJDJb_VrNNs1k4grsfyFv0HfYv0Kr9v4sChQ@mail.gmail.com>
In-Reply-To: <20130411110324.GD12770@suse.cz>

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.