On Fri, Jul 11, 2014 at 11:45:58AM +0100, Ramsay Jones wrote:

> > @@ -1729,9 +1729,8 @@ static enum peel_status peel_object(const unsigned 
> > char *name, unsigned char *sh
> >  
> >     if (o->type == OBJ_NONE) {
> >             int type = sha1_object_info(name, NULL);
> > -           if (type < 0)
> > +           if (type < 0 || !object_as_type(o, type, 0))
> --------------------------------------------------------^^^
> 
> It is not possible here for object_as_type() to issue an error()
> report, but I had to go look at the code to check. (It would have
> been a change in behaviour, otherwise). So, it doesn't really matter
> what you pass to the quiet argument, but setting it to 1 _may_ help
> the next reader. (No, I'm not convinced either; the only reason I
> knew it had anything to do with error messages was because I had
> just read the code ...) Hmm, dunno.

Right, as I mentioned in the commit message, the type-check part of
object_type is a noop here. In that sense you could write this as just:

  object_as_type(o, type. 1);

and ignore the return value. However, I'd rather err on the side of
checking for and reporting the error, even if we expect it to do nothing
right now. That decouples the two functions, and if object_as_type ever
learns to report other errors, then we automatically do the right
thing here.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to