Re: [PATCHv6 06/23] notes.h/c: Propagate combine_notes_fn return value to add_note() and beyond

Thiago Farina wrote:

On Tue, Nov 9, 2010 at 7:49 PM, Johan Herland [off-list ref] wrote:

quoted

--- a/builtin/notes.c
+++ b/builtin/notes.c

@@ -573,8 +573,8 @@ static int add(int argc, const char **argv, const char *prefix)

       if (is_null_sha1(new_note))
               remove_note(t, object);
-       else
-               add_note(t, object, new_note, combine_notes_overwrite);
+       else if (add_note(t, object, new_note, combine_notes_overwrite))
+               die("confused: combine_notes_overwrite failed");

This message looks like more a debug information. Could it be
rewritten to be more user friendly / more informative?

Sverre mentioned the same thing.  The intent is the same as the existing

		die("should not happen, someone must be hit on the forehead");

--- i.e., to provoke bug reports if it happens, without the appearance

of begging. :)

So how about:

		die("BUG: combine_notes_overwrite failed");

Something involving assert() could also work, but that tends to be
a bit ugly.