On Mon, Nov 07 2022, Teng Long wrote:
> From: Teng Long <dyroneteng@xxxxxxxxx>
>
> Situation of removing note shouldn't happen in "append_edit()",
> unless it's a bug. So, let's drop the unreachable "else" code
> in "append_edit()".
>
> Signed-off-by: Teng Long <dyroneteng@xxxxxxxxx>
> ---
> builtin/notes.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/builtin/notes.c b/builtin/notes.c
> index 02b88e54d8..6d42592c79 100644
> --- a/builtin/notes.c
> +++ b/builtin/notes.c
> @@ -10,6 +10,7 @@
> #include "cache.h"
> #include "config.h"
> #include "builtin.h"
> +#include "git-compat-util.h"
Huh? cache.h includes this already, why are we doing it again?
> #include "notes.h"
> #include "object-store.h"
> #include "repository.h"
> @@ -646,11 +647,7 @@ static int append_edit(int argc, const char **argv, const char *prefix)
> _("Both original and appended notes are empty in %s, do nothing\n"),
> oid_to_hex(&object));
> } else {
> - fprintf(stderr, _("Removing note for object %s\n"),
> - oid_to_hex(&object));
> - remove_note(t, object.hash);
> - logmsg = xstrfmt("Notes removed by 'git notes %s'", argv[0]);
> - commit_notes(the_repository, t, logmsg);
> + BUG("compute_notes failed");
> }
Let's squash this into 2/3. So you were adding to it then, but we didn't
need to add this commit_notes() at all?
Also, there you did:
- char *logmsg;
+ char *logmsg = NULL;
But here we see that it's only used in the initial "if" arm, so the
reason to have it declared earlier has disappeared in this 3/3. Instead
just have 2/3 do:
if (...) {
char *logmsg;
[...]
logmsg = xstrfmt(...);
commit_notes(....);
free(logmsg);
} else if (...) {
...
}
This commit also shows that there wasn't any reason for your "struct
note_data d" cleanup, i.e. we weren't adding a field or anything...
