From: Jonathan Nieder <jrnieder@gmail.com>
To: Johan Herland <johan@herland.net>
Cc: git@vger.kernel.org, bebarino@gmail.com, gitster@pobox.com
Subject: Re: [PATCH 04/18] notes.h/c: Propagate combine_notes_fn return value to add_note() and beyond
Date: Tue, 5 Oct 2010 10:38:38 -0500 [thread overview]
Message-ID: <20101005153838.GG12797@burratino> (raw)
In-Reply-To: <1285719811-10871-5-git-send-email-johan@herland.net>
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?
next prev parent reply other threads:[~2010-10-05 15:42 UTC|newest]
Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-09-29 0:23 [PATCH 00/18] git notes merge Johan Herland
2010-09-29 0:23 ` [PATCH 01/18] (trivial) notes.h: Minor documentation fixes to copy_notes() Johan Herland
2010-10-05 14:55 ` Jonathan Nieder
2010-10-05 15:22 ` Johan Herland
2010-10-05 15:26 ` Jonathan Nieder
2010-09-29 0:23 ` [PATCH 02/18] notes.h: Make default_notes_ref() available in notes API Johan Herland
2010-09-29 0:23 ` [PATCH 03/18] notes.h/c: Clarify the handling of notes objects that are == null_sha1 Johan Herland
2010-10-05 15:21 ` Jonathan Nieder
2010-10-05 22:30 ` Johan Herland
2010-09-29 0:23 ` [PATCH 04/18] notes.h/c: Propagate combine_notes_fn return value to add_note() and beyond Johan Herland
2010-10-05 15:38 ` Jonathan Nieder [this message]
2010-10-06 19:40 ` Johan Herland
2010-09-29 0:23 ` [PATCH 05/18] (trivial) t3303: Indent with tabs instead of spaces for consistency Johan Herland
2010-09-29 0:23 ` [PATCH 06/18] notes.c: Use two newlines (instead of one) when concatenating notes Johan Herland
2010-09-29 0:23 ` [PATCH 07/18] builtin/notes.c: Split notes ref DWIMmery into a separate function Johan Herland
2010-10-05 15:50 ` Jonathan Nieder
2010-10-07 13:56 ` Johan Herland
2010-09-29 0:23 ` [PATCH 08/18] git notes merge: Initial implementation handling trivial merges only Johan Herland
2010-10-07 4:37 ` Test script style (Re: [PATCH 08/18] git notes merge: Initial implementation handling trivial merges only) Jonathan Nieder
2010-10-07 4:57 ` Junio C Hamano
2010-10-07 6:24 ` [PATCH 08/18] git notes merge: Initial implementation handling trivial merges only Jonathan Nieder
2010-10-08 23:55 ` Johan Herland
2010-10-09 17:29 ` Jonathan Nieder
2010-10-21 2:12 ` Johan Herland
2010-09-29 0:23 ` [PATCH 09/18] builtin/notes.c: Refactor creation of notes commits Johan Herland
2010-09-29 0:23 ` [PATCH 10/18] git notes merge: Handle real, non-conflicting notes merges Johan Herland
2010-09-29 16:20 ` Ævar Arnfjörð Bjarmason
2010-09-29 23:25 ` Johan Herland
2010-09-29 0:23 ` [PATCH 11/18] git notes merge: Add automatic conflict resolvers (ours, theirs, union) Johan Herland
2010-10-02 9:14 ` Stephen Boyd
2010-10-04 15:10 ` Johan Herland
2010-09-29 0:23 ` [PATCH 12/18] Documentation: Preliminary docs on 'git notes merge' Johan Herland
2010-10-02 8:55 ` Stephen Boyd
2010-10-04 15:15 ` Johan Herland
2010-09-29 0:23 ` [PATCH 13/18] git notes merge: Manual conflict resolution, part 1/2 Johan Herland
2010-09-29 0:23 ` [PATCH 14/18] git notes merge: Manual conflict resolution, part 2/2 Johan Herland
2010-09-29 16:19 ` Ævar Arnfjörð Bjarmason
2010-09-29 23:37 ` Johan Herland
2010-09-29 0:23 ` [PATCH 15/18] git notes merge: List conflicting notes in notes merge commit message Johan Herland
2010-09-29 0:23 ` [PATCH 16/18] git notes merge: --commit should fail if underlying notes ref has moved Johan Herland
2010-09-29 0:23 ` [PATCH 17/18] git notes merge: Add another auto-resolving strategy: "cat_sort_uniq" Johan Herland
2010-09-29 0:23 ` [PATCH 18/18] git notes merge: Add testcases for merging notes trees at different fanouts Johan Herland
2010-09-29 14:56 ` [PATCH 00/18] git notes merge Sverre Rabbelier
2010-09-29 15:16 ` Johan Herland
2010-09-29 15:20 ` Sverre Rabbelier
2010-09-29 16:04 ` Johan Herland
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20101005153838.GG12797@burratino \
--to=jrnieder@gmail.com \
--cc=bebarino@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=johan@herland.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).