On Thu, Apr 11, 2013 at 03:52:44PM +0530, Ramkumar Ramachandra <artagnon@xxxxxxxxx> 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. :-)
Attachment:
signature.asc
Description: Digital signature