Hi Johannes On 2015-10-06 at 16:30:36 +0200, Johannes Schindelin <johannes.schinde...@gmx.de> wrote: > On 2015-10-06 15:51, Tobias Klauser wrote: > > > On 2015-10-06 at 15:16:12 +0200, Johannes Schindelin > > <johannes.schinde...@gmx.de> wrote: > >> > >> On 2015-10-06 14:15, Tobias Klauser wrote: > >> > prented_sha1_file() always returns 0 and its only callsite in > >> > builtin/blame.c doesn't use the return value, so change the return type > >> > to void. > >> > >> While this commit message is technically correct, it would appear that > >> there are some things left unsaid. > >> > >> Is there a problem with the current code that is solved by not returning > >> 0? If so, could you add it to the commit message? And in particular, > >> change the oneline appropriately? > > > > There's no problem with the current code other than that the return > > value is unused and thus unnecessary for correct funcionality. So it's > > certainly not a functional problem but rather a cosmetic change. > > Okay. > > > Does such a change even make sense (it's one of my first patch to git, > > so I'm not really sure what your criteria in this respect are)? > > Welcome! > > As to the patch, I cannot speak for Junio, of course, but my preference would > be to keep the return type. Traditionally, functions that can fail either > die() or return an int; non-zero indicates an error. In this case, it seems > that we do not have any condition (yet...) under which an error could occur. > It does not seem very unlikely that we may eventually have such conditions, > though, hence my preference.
Ok, I see. Thank you for your explanation. I'll wait for Junio's decision then :) Cheers Tobias -- 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
