git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Thiago Farina <tfransosi@gmail.com>
Date
Nov 14, 2010, 20:46 UTC
Message-ID
<AANLkTinQhkXvfa0BS0dFK6-mXFpU+uQ=rKRJ1QaXXA1S@mail.gmail.com>
In-Reply-To
<1289339399-4733-7-git-send-email-johan@herland.net>
On Tue, Nov 9, 2010 at 7:49 PM, Johan Herland <johan@herland.net> wrote:
Show 37 quoted lines
> 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().
>
> Instead, propagate this return value out to add_note(), and return it
> from there to enable the caller to handle errors appropriately.
>
> Existing add_note() callers are updated to die() upon failure, thus
> preserving the current behaviour. The only exceptions are copy_note()
> and notes_cache_put() where we are able to propagate the add_note()
> return value instead.
>
> This patch has been improved by the following contributions:
> - Jonathan Nieder: Future-proof by always checking add_note() return value
> - Jonathan Nieder: Improve clarity of final if-condition in note_tree_insert()
>
> Thanks-to: Jonathan Nieder <jrnieder@gmail.com>
> Signed-off-by: Johan Herland <johan@herland.net>
> ---
>  builtin/notes.c |   11 ++++++-----
>  notes-cache.c   |    3 +--
>  notes.c         |   55 ++++++++++++++++++++++++++++---------------------------
>  notes.h         |   11 ++++++++---
>  4 files changed, 43 insertions(+), 37 deletions(-)
>
> diff --git a/builtin/notes.c b/builtin/notes.c
> index fbc347c..35f6eb6 100644
> --- 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?

Previous: Johan HerlandNext: Jonathan Nieder
Message 8 of 31 in “[PATCHv6 00/23] git notes merge”
  1. Johan HerlandNov 9, 2010
  2. 01/23 notes.c: Hexify SHA1 in die() message from init_notes()Johan Herland, Nov 9, 2010
  3. 02/23 (trivial) notes.h: Minor documentation fixes to copy_notes()Johan Herland, Nov 9, 2010
  4. 03/23 notes.h: Make default_notes_ref() available in notes APIJohan Herland, Nov 9, 2010
  5. 04/23 notes.c: Reorder functions in preparation for next commitJohan Herland, Nov 9, 2010
  6. 05/23 notes.h/c: Allow combine_notes functions to remove notesJohan Herland, Nov 9, 2010
  7. 06/23 notes.h/c: Propagate combine_notes_fn return value to add_note() and beyondJohan Herland, Nov 9, 2010
  8. Thiago FarinaNov 14, 2010
  9. Jonathan NiederNov 14, 2010
  10. Johan HerlandNov 14, 2010
  11. 06/23 notes.h/c: Propagate combine_notes_fn return value to add_note() and beyondJohan Herland, Nov 14, 2010
  12. 12/23 git notes merge: Handle real, non-conflicting notes mergesJohan Herland, Nov 14, 2010
  13. 13/23 git notes merge: Add automatic conflict resolvers (ours, theirs, union)Johan Herland, Nov 14, 2010
  14. 19/23 git notes merge: Add another auto-resolving strategy: "cat_sort_uniq"Johan Herland, Nov 14, 2010
  15. 07/23 (trivial) t3303: Indent with tabs instead of spaces for consistencyJohan Herland, Nov 9, 2010
  16. 08/23 notes.c: Use two newlines (instead of one) when concatenating notesJohan Herland, Nov 9, 2010
  17. 09/23 builtin/notes.c: Split notes ref DWIMmery into a separate functionJohan Herland, Nov 9, 2010
  18. 10/23 git notes merge: Initial implementation handling trivial merges onlyJohan Herland, Nov 9, 2010
  19. 11/23 builtin/notes.c: Refactor creation of notes commits.Johan Herland, Nov 9, 2010
  20. 12/23 git notes merge: Handle real, non-conflicting notes mergesJohan Herland, Nov 9, 2010
  21. 13/23 git notes merge: Add automatic conflict resolvers (ours, theirs, union)Johan Herland, Nov 9, 2010
  22. 14/23 Documentation: Preliminary docs on 'git notes merge'Johan Herland, Nov 9, 2010
  23. 15/23 git notes merge: Manual conflict resolution, part 1/2Johan Herland, Nov 9, 2010
  24. 16/23 git notes merge: Manual conflict resolution, part 2/2Johan Herland, Nov 9, 2010
  25. 17/23 git notes merge: List conflicting notes in notes merge commit messageJohan Herland, Nov 9, 2010
  26. 18/23 git notes merge: --commit should fail if underlying notes ref has movedJohan Herland, Nov 9, 2010
  27. 19/23 git notes merge: Add another auto-resolving strategy: "cat_sort_uniq"Johan Herland, Nov 9, 2010
  28. 20/23 git notes merge: Add testcases for merging notes trees at different fanoutsJohan Herland, Nov 9, 2010
  29. 21/23 Provide 'git notes get-ref' to easily retrieve current notes refJohan Herland, Nov 9, 2010
  30. 22/23 cmd_merge(): Parse options before checking MERGE_HEADJohan Herland, Nov 9, 2010
  31. 23/23 Provide 'git merge --abort' as a synonym to 'git reset --merge'Johan Herland, Nov 9, 2010

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.