mbox series

[0/3] libify reflog

Message ID pull.1218.git.git.1645209647.gitgitgadget@gmail.com (mailing list archive)
Headers show
Series libify reflog | expand

Message

Koji Nakamaru via GitGitGadget Feb. 18, 2022, 6:40 p.m. UTC
In [1], there was a discussion around a bug report of stash not recovering
in the middle of the process. It turned out to not be a bug we need to fix.
However, out of that discussion came the idea of libifying reflog. This can
stand alone as a code improvement.

stash.c currently shells out to call reflog to delete reflogs. Libify reflog
delete and call it from both builtin/reflog.c and builtin/stash.c.

This patch has three parts:

 * libify reflog's delete functionality and move some of the helpers into a
   reflog.c library and export them
 * call reflog_delete from builtin/reflog.c
 * call reflog_delete from builtin/stash.c

 1. https://lore.kernel.org/git/220126.86h79qe692.gmgdl@evledraar.gmail.com/

John Cai (3):
  reflog: libify delete reflog function and helpers
  reflog: call reflog_delete from reflog.c
  stash: call reflog_delete from reflog.c

 Makefile         |   1 +
 builtin/reflog.c | 451 +----------------------------------------------
 builtin/stash.c  |  16 +-
 reflog.c         | 432 +++++++++++++++++++++++++++++++++++++++++++++
 reflog.h         |  49 +++++
 5 files changed, 490 insertions(+), 459 deletions(-)
 create mode 100644 reflog.c
 create mode 100644 reflog.h


base-commit: b80121027d1247a0754b3cc46897fee75c050b44
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1218%2Fjohn-cai%2Fjc-libify-reflog-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1218/john-cai/jc-libify-reflog-v1
Pull-Request: https://github.com/git/git/pull/1218

Comments

Ævar Arnfjörð Bjarmason Feb. 18, 2022, 7:29 p.m. UTC | #1
On Fri, Feb 18 2022, John Cai via GitGitGadget wrote:

> In [1], there was a discussion around a bug report of stash not recovering
> in the middle of the process. It turned out to not be a bug we need to fix.
> However, out of that discussion came the idea of libifying reflog. This can
> stand alone as a code improvement.
>
> stash.c currently shells out to call reflog to delete reflogs. Libify reflog
> delete and call it from both builtin/reflog.c and builtin/stash.c.
>
> This patch has three parts:
>
>  * libify reflog's delete functionality and move some of the helpers into a
>    reflog.c library and export them
>  * call reflog_delete from builtin/reflog.c
>  * call reflog_delete from builtin/stash.c
>
>  1. https://lore.kernel.org/git/220126.86h79qe692.gmgdl@evledraar.gmail.com/

I just reviewed this in detail, it's very nice to see this done &
another shell-out in our code API-ified.

There's at least one bad bug here (but easily fixed), and some nits
etc. that I suggested mainly to make the diff easier to look at locally,
but aside from those minor adjustments this looks very good to me.