On Thu, Apr 11, 2013 at 03:52:44PM +0530, Ramkumar Ramachandra
<artag...@gmail.com> wrote:
> > + for (i = 0; i < opts->revs->pending.nr; i++) {
> > + unsigned char sha1[20];
> > + const char *name = opts->revs->pending.objects[i].name;
> > +
> > + if (!get_sha1(name, sha1)) {
> > + enum object_type type = sha1_object_info(sha1,
> > NULL);
> > +
> > + if (type > 0 && type != OBJ_COMMIT)
> > + die(_("%s: can't cherry-pick a %s"), name,
> > typename(type));
> > + }
>
> else? What happens if get_sha1() fails?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. > > diff --git a/t/t3508-cherry-pick-many-commits.sh > > b/t/t3508-cherry-pick-many-commits.sh > > index 4e7136b..19c99d7 100755 > > --- a/t/t3508-cherry-pick-many-commits.sh > > +++ b/t/t3508-cherry-pick-many-commits.sh > > @@ -55,6 +55,12 @@ one > > two" > > ' > > > > +test_expect_success 'cherry-pick three one two: fails' ' > > + git checkout -f master && > > + git reset --hard first && > > + test_must_fail git cherry-pick three one two: > > +' > > So you're testing just the third case (where commit objects are mixed > with non-commit objects), which is arguably a bug. Okay. 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. :-)
signature.asc
Description: Digital signature
