On Fri, Feb 18, 2022 at 06:40:45PM +0000, John Cai via GitGitGadget wrote:
> From: John Cai <johncai86@xxxxxxxxx>
>
> Currently stash shells out to reflog in order to delete refs. In an
> effort to reduce how much we shell out to a subprocess, libify the
> functionality that stash needs into reflog.c.
Sounds great. For other reviewers, looking at this with `--color-moved`
if you have the patches applied locally makes it much easier to see what
is going on here, which is basically:
- All of the implementation that used to be in builtin/reflog.c moved
to reflog.c.
- The function signatures and structure declarations moved to
reflog.h.
> Add a reflog_delete function that is pretty much the logic in the while
> loop in builtin/reflog.c cmd_reflog_delete(). This is a function that
> builtin/reflog.c and builtin/stash.c can both call.
As you mentioned, the `reflog_delete()` implementation is indeed new. It
looks like Ævar reviewed most of it already, so I'll take a look at his
message before I end up repeating everything he already said ;).
It's worth noting that the subsequent clean-up to rewrite
cmd_reflog_delete() in terms of your new `reflog_delete()` happens in
the subsequent commit. If you end up rerolling this series, mentioning
that here may be worthwhile.
One question that I had which I don't see answered already is what the
plan is for existing reflog-related functions that live in refs.h.
Should or will those functions be moved to the new reflog-specific
header, too?
Thanks,
Taylor
