On Tue, Dec 21, 2021 at 1:28 PM Ævar Arnfjörð Bjarmason
<avarab@xxxxxxxxx> wrote:
>
>
> On Tue, Dec 21 2021, Elijah Newren via GitGitGadget wrote:
>
> > From: Elijah Newren <newren@xxxxxxxxx>
>
> > + if (rev->remerge_diff) {
> > + rev->remerge_objdir = tmp_objdir_create("remerge-diff");
> > + if (!rev->remerge_objdir)
> > + die(_("unable to create temporary object directory"));
>
> It looks like the tmp_objdir_create() API is rather bad about mixing
> errors that would come with an errno with others, but shouldn't this be
> die_errno() in the case where it would fail due to a syscall? Even
> better would be passing a "gentle" to it and have it emit the
> appropriate errors.
I can switch to die_errno().
>
> > + if (rev.remerge_diff)
> > + die(_("--remerge_diff does not make sense"));
>
> s/_/-/
Indeed, thanks.
> > + struct merge_options o;
> > + struct commit_list *bases;
> > + struct merge_result res;
>
> nit: could use "= { 0 }" instead of memset below.
Sure, I can make that change.
> > + /* Re-merge the parents */
> > + merge_incore_recursive(&o,
> > + bases, parents->item, parents->next->item,
> > + &res);
>
> style: odd not to have arguments that fit on the line on
> the line, i.e. "&o, bases, ...".
Yes, but this groups all the ancestors so nicely as opposed to the
typical happenstance of whatever fits on a line. ;-)
> > + /* Clean up the temporary object directory */
> > + if (opt->remerge_objdir != NULL)
>
> style: if (!x) not if (x != NULL)
Ok, will change.
