From: Johan Herland <johan@herland.net>
To: Jonathan Nieder <jrnieder@gmail.com>
Cc: git@vger.kernel.org, bebarino@gmail.com, avarab@gmail.com,
gitster@pobox.com
Subject: Re: [PATCHv4 06/21] notes.h/c: Propagate combine_notes_fn return value to add_note() and beyond
Date: Thu, 21 Oct 2010 15:16:22 +0200 [thread overview]
Message-ID: <201010211516.23091.johan@herland.net> (raw)
In-Reply-To: <20101021052100.GB2413@burratino>
On Thursday 21 October 2010, Jonathan Nieder wrote:
> Johan Herland wrote:
> > --- a/notes.c
> > +++ b/notes.c
> > @@ -303,13 +298,17 @@ static void note_tree_insert(struct
> > notes_tree *t, struct int_node *tree, GET_PTR_TYPE(*p) ==
> > PTR_TYPE_SUBTREE);
> > if (is_null_sha1(entry->val_sha1)) { /* skip insertion of empty
> > note */ free(entry);
> > - return;
> > + return 0;
> > }
> > new_node = (struct int_node *) xcalloc(sizeof(struct int_node),
> > 1); - note_tree_insert(t, new_node, n + 1, l, GET_PTR_TYPE(*p),
> > - combine_notes);
> > - *p = SET_PTR_TYPE(new_node, PTR_TYPE_INTERNAL);
> > - note_tree_insert(t, new_node, n + 1, entry, type, combine_notes);
> > + ret = note_tree_insert(t, new_node, n + 1, l, GET_PTR_TYPE(*p),
> > + combine_notes);
> > + if (!ret) {
> > + *p = SET_PTR_TYPE(new_node, PTR_TYPE_INTERNAL);
> > + ret = note_tree_insert(t, new_node, n + 1, entry, type,
> > + combine_notes);
> > + }
> > + return ret;
>
> Micronit: it would probably be clearer to write
>
> if (ret)
> return ret;
> *p = SET_PTR_TYPE(...
> return note_tree_insert(...
>
> to avoid a little nesting.
Agreed. Will be fixed in the next iteration.
Thanks,
...Johan
--
Johan Herland, <johan@herland.net>
www.herland.net
next prev parent reply other threads:[~2010-10-21 13:17 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-10-21 2:08 [PATCHv4 00/21] git notes merge Johan Herland
2010-10-21 2:08 ` [PATCHv4 01/21] notes.c: Hexify SHA1 in die() message from init_notes() Johan Herland
2010-10-21 2:08 ` [PATCHv4 02/21] (trivial) notes.h: Minor documentation fixes to copy_notes() Johan Herland
2010-10-21 2:08 ` [PATCHv4 03/21] notes.h: Make default_notes_ref() available in notes API Johan Herland
2010-10-21 2:08 ` [PATCHv4 04/21] notes.c: Reorder functions in preparation for next commit Johan Herland
2010-10-21 2:08 ` [PATCHv4 05/21] notes.h/c: Clarify the handling of notes objects that are == null_sha1 Johan Herland
2010-10-21 5:12 ` Jonathan Nieder
2010-10-21 13:13 ` Johan Herland
2010-10-21 17:54 ` Jonathan Nieder
2010-10-22 13:32 ` Johan Herland
2010-10-21 2:08 ` [PATCHv4 06/21] notes.h/c: Propagate combine_notes_fn return value to add_note() and beyond Johan Herland
2010-10-21 5:21 ` Jonathan Nieder
2010-10-21 13:16 ` Johan Herland [this message]
2010-10-21 2:08 ` [PATCHv4 07/21] (trivial) t3303: Indent with tabs instead of spaces for consistency Johan Herland
2010-10-21 2:08 ` [PATCHv4 08/21] notes.c: Use two newlines (instead of one) when concatenating notes Johan Herland
2010-10-21 2:08 ` [PATCHv4 09/21] builtin/notes.c: Split notes ref DWIMmery into a separate function Johan Herland
2010-10-21 2:08 ` [PATCHv4 10/21] git notes merge: Initial implementation handling trivial merges only Johan Herland
2010-10-21 2:08 ` [PATCHv4 11/21] builtin/notes.c: Refactor creation of notes commits Johan Herland
2010-10-21 2:08 ` [PATCHv4 12/21] git notes merge: Handle real, non-conflicting notes merges Johan Herland
2010-10-21 2:08 ` [PATCHv4 13/21] git notes merge: Add automatic conflict resolvers (ours, theirs, union) Johan Herland
2010-10-21 2:08 ` [PATCHv4 14/21] Documentation: Preliminary docs on 'git notes merge' Johan Herland
2010-10-21 2:08 ` [PATCHv4 15/21] git notes merge: Manual conflict resolution, part 1/2 Johan Herland
2010-10-21 2:08 ` [PATCHv4 16/21] git notes merge: Manual conflict resolution, part 2/2 Johan Herland
2010-10-21 2:08 ` [PATCHv4 17/21] git notes merge: List conflicting notes in notes merge commit message Johan Herland
2010-10-21 2:08 ` [PATCHv4 18/21] git notes merge: --commit should fail if underlying notes ref has moved Johan Herland
2010-10-21 2:08 ` [PATCHv4 19/21] git notes merge: Add another auto-resolving strategy: "cat_sort_uniq" Johan Herland
2010-10-21 2:08 ` [PATCHv4 20/21] git notes merge: Add testcases for merging notes trees at different fanouts Johan Herland
2010-10-21 2:08 ` [PATCHv4 21/21] Provide 'git notes get-ref' to easily retrieve current notes ref Johan Herland
2010-10-21 21:00 ` [PATCHv4 00/21] git notes merge Sverre Rabbelier
2010-10-21 23:20 ` Junio C Hamano
2010-10-21 23:30 ` Jonathan Nieder
2010-10-22 14:11 ` git merge --abort? [was: Re: [PATCHv4 00/21] git notes merge] Johan Herland
2010-10-22 14:55 ` Jonathan Nieder
2010-10-22 15:12 ` Johan Herland
2010-10-22 15:20 ` Sverre Rabbelier
2010-10-22 15:48 ` Jonathan Nieder
2010-10-22 15:57 ` Sverre Rabbelier
2010-10-22 15:41 ` [PATCHv4 00/21] git notes merge Johan Herland
2010-10-22 15:54 ` Sverre Rabbelier
2010-10-23 0:47 ` Johan Herland
2010-10-23 1:44 ` Sverre Rabbelier
2010-10-22 22:28 ` Johan Herland
2010-10-23 1:38 ` Sverre Rabbelier
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=201010211516.23091.johan@herland.net \
--to=johan@herland.net \
--cc=avarab@gmail.com \
--cc=bebarino@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=jrnieder@gmail.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.