On Fri, Feb 9, 2024 at 12:06 AM Johannes Schindelin <Johannes.Schindelin@xxxxxx> wrote: > > Hi Kyle, > > On Mon, 5 Feb 2024, Kyle Lippincott wrote: > > > On Mon, Feb 5, 2024 at 6:36 AM Johannes Schindelin via GitGitGadget > > <gitgitgadget@xxxxxxxxx> wrote: > > > > > > diff --git a/builtin/notes.c b/builtin/notes.c > > > index e65cae0bcf7..caf20fd5bdd 100644 > > > --- a/builtin/notes.c > > > +++ b/builtin/notes.c > > > @@ -716,9 +716,11 @@ static int append_edit(int argc, const char **argv, const char *prefix) > > > struct strbuf buf = STRBUF_INIT; > > > char *prev_buf = repo_read_object_file(the_repository, note, &type, &size); > > > > > > - if (prev_buf && size) > > > + if (!prev_buf) > > > + die(_("unable to read %s"), oid_to_hex(note)); > > > > This changes the behavior of this function. Previously, it would not > > add prev_buf output, but still succeed. This now dies. > > It does change behavior. The previous behavior looked up the note OID, > then tried to read it, and if it was missing just pretended that there had > not been a note. > > I'm not quite sure whether we should keep that behavior, as it is unclear > in which scenarios it would be desirable to paper over missing objects. > > In GitGitGadget, I am a heavy user of notes and it wouldn't do any good to > have this behavior: It would lose information. > > And even in scenarios where the `notes` ref is fetch shallowly, I would > expect all of the actual notes blobs to be present, and I would _want_ the > `git note edit ...` command to error out when that blob is not found. > > Does that reasoning make sense to you? Sounds good, thanks for talking through it with me. > > Ciao, > Johannes