On Tuesday 5. October 2010 17.38.38 Jonathan Nieder wrote: > Johan Herland wrote: > > The combine_notes_fn functions uses a non-zero return value to indicate > > failure. However, this return value was converted to a call to die() > > in note_tree_insert(). > > For what it's worth, I still like it. > > > Instead, propagate this return value out to add_note(), and return it > > from there > > It's always odd deciding what to do in the code paths that "can't > happen". Ideally one would want static analyzers to check that, while > at the same time subjecting the user to some nice graceful fallback > behavior when it happens anyway. > > fatal: confused: combine_notes_overwrite failed > > In this case I'm not sure what the message in die() should be, but > it seems sane to die with some message. Maybe this should be > mentioned in the commit message, though? Ok, I've added a paragraph to the commit message mentioning that most of add_note()'s callers now implement the die() previously located in notes_tree_insert(). Will be in the next iteration. Thanks, ...Johan -- Johan Herland, <johan@xxxxxxxxxxx> www.herland.net -- 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