On Wed, Oct 7, 2015 at 2:14 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > Stefan Beller <sbeller@xxxxxxxxxx> writes: > >> Compare to a patch[1] I sent a while back and the discussion on it. >> >> [1] https://www.mail-archive.com/git@xxxxxxxxxxxxxxx/msg70474.html > > It is not clear what conclusion you want others to draw from the > comparison, I am afraid. I did not draw a conclusion. All I wanted is to point out, we've had similar patches before. Maybe I wanted to point out, you had a different opinion about the patch I linked to than you seem to have now about this patch. > > I am guessing that you are in favor of dropping this patch, because > 'int' that signals success or error is the most natural return type > and meanint for this function if its callers ever start using the > value as the indication of an error, just like in the old thread, > the return value from get_remote_heads() had the most useful type > and the meaning for its callers if they wanted to use it. > > And if that is what you wanted to say, I fully agree with the > conclusion. I really did not want to say anything except for pointing out how similar cases were dealt with in the past. So I guess for a good comparision we'd need to asses how similar the patches are. If they are similar it's easier to link to the old discussion instead of retyping the same reasons. > > By the way, it is not a very good comparison, though. The patch in > the old thread deliberately attempted to discard a useful piece of > information. The information the patch in this thread attempts to > discard is not so useful, as there currently is nobody that returns > an error in the codepath. Isn't that a bit picky? (old thread: the information is useful, but nobody uses it, this thread: information is useless, and nobody uses it) So the similarity is nobody is using the result, the difference is the usefulness of the information provided. > So in that sense, the patch in this > thread to change the return value to void is a bit more justifiable > than the one in the old thread, I think. That makes sense to me. > > Thanks. -- 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