Subject: Re: [PATCH 1/3] reflog: libify delete reflog function and helpers

From: Junio C Hamano <[email-masked]>
Date: Fri, 18 Feb 2022 12:00:44 -0800

"John Cai via GitGitGadget" <[email-masked]> writes:

> diff --git a/reflog.h b/reflog.h
> new file mode 100644
> index 00000000000..e4a8a104f45
> --- /dev/null
> +++ b/reflog.h
> @@ -0,0 +1,49 @@
> +#ifndef REFLOG_H
> +#define REFLOG_H
> +
> +#include "cache.h"
> +#include "commit.h"
> +
> +/* Remember to update object flag allocation in object.h */
> +#define INCOMPLETE	(1u<<10)
> +#define STUDYING	(1u<<11)
> +#define REACHABLE	(1u<<12)

These names were unique enough back when they were part of internal
implementation detail inside builtin/reflog.c but it no longer is in
the scope it is visible.  builtin/stash.c (and whatever other things
that may want to deal with reflogs in the future) is about stash
entries and wants to have a way to differentiate these symbols from
others and hint that these are about reflog operation.

Perhaps prefix them with REFLOG_ or something?

Or, do we even NEED to expose these bits outside the implementation
of reflog.c?  If these three constants are used ONLY by reflog.c and
not by builtin/reflog.c (or builtin/stash.c), then moving them to
where they are used, i.e. in reflog.c, without exposing them to
others in reflog.h, would be a far better thing to do.  That way,
they can stay with their original name, without having to add a
differentiating prefix.

I also checked if any of the following struct's can be left as a
private implementation detail, but they are shared between reflog.c
and builtin/reflog.c and need to be in reflog.h

> +struct cmd_reflog_expire_cb {
> +	int stalefix;
> +	int explicit_expiry;
> +	timestamp_t expire_total;
> +	timestamp_t expire_unreachable;
> +	int recno;
> +};
> +
> +struct expire_reflog_policy_cb {
> +	enum {
> +		UE_NORMAL,
> +		UE_ALWAYS,
> +		UE_HEAD
> +	} unreachable_expire_kind;
> +	struct commit_list *mark_list;
> +	unsigned long mark_limit;
> +	struct cmd_reflog_expire_cb cmd;
> +	struct commit *tip_commit;
> +	struct commit_list *tips;
> +	unsigned int dry_run:1;
> +};
> +
> +int reflog_delete(const char*, int, int);
> +void reflog_expiry_cleanup(void *);
> +void reflog_expiry_prepare(const char*, const struct object_id*,
> +			   void *);
> +int should_expire_reflog_ent(struct object_id *, struct object_id*,
> +				    const char *, timestamp_t, int,
> +				    const char *, void *);
> +int count_reflog_ent(struct object_id *ooid, struct object_id *noid,
> +		const char *email, timestamp_t timestamp, int tz,
> +		const char *message, void *cb_data);
> +int should_expire_reflog_ent_verbose(struct object_id *,
> +				     struct object_id *,
> +				     const char *,
> +				     timestamp_t, int,
> +				     const char *, void *);
> +#endif /* REFLOG_H */