From: Junio C Hamano <gitster@pobox.com>
Date: Thu, 25 Jun 2015 21:01:36 GMT
Subject: Re: [PATCH v2 5/6] git-reflog: add create and exists functions
Message-ID: <xmqq1tgzy027.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1435265110-6414-5-git-send-email-dturner@twopensource.com>

David Turner <dturner@twopensource.com> writes:

Because...???

> These are necessary because ref backends manage reflogs.

"Because ref backends manage reflogs" may be a good explanation to
implement something as part of ref API, but it does not explain why
that something needs to be implemented in the first place.

> In a moment,
> we will use these functions to make git stash work with alternate
> ref backends.
>
> Signed-off-by: David Turner <dturner@twopensource.com>
> ---
>  builtin/reflog.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 78 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/reflog.c b/builtin/reflog.c
> index c2eb8ff..a64158d 100644
> --- a/builtin/reflog.c
> +++ b/builtin/reflog.c
> @@ -13,6 +13,10 @@ static const char reflog_expire_usage[] =
>  "git reflog expire [--expire=<time>] [--expire-unreachable=<time>] [--rewrite] [--updateref] [--stale-fix] [--dry-run | -n] [--verbose] [--all] <refs>...";
>  static const char reflog_delete_usage[] =
>  "git reflog delete [--rewrite] [--updateref] [--dry-run | -n] [--verbose] <refs>...";
> +static const char reflog_create_usage[] =
> +"git reflog create <refs>...";

It makes sense to take more than one; you may want to prepare more
than one, and you would signal error by exiting with a non-zero
status if any of them failed.

> +static const char reflog_exists_usage[] =
> +"git reflog exists <refs>...";

This is iffy, though.  "git reflog exists foo bar" says "all of them
exist" or "some of them exist"?  Why is it more useful to implement
"all of them exist"?

Perhaps dropping "..." would be simpler to explain and less
confusing.

> @@ -699,12 +703,79 @@ static int cmd_reflog_delete(int argc, const char **argv, const char *prefix)
>  	return status;
>  }
>  
> +static int cmd_reflog_create(int argc, const char **argv, const char *prefix)
> +{
> +	int i, status = 0, start = 0;
> +	struct strbuf err = STRBUF_INIT;
> +
> +	for (i = 1; i < argc; i++) {
> +		const char *arg = argv[i];
> +		if (!strcmp(arg, "--")) {
> +			i++;
> +			break;
> +		}
> +		else if (arg[0] == '-')
> +			usage(reflog_create_usage);
> +		else
> +			break;
> +	}
> +
> +	start = i;
> +
> +	if (argc - start < 1)
> +		return error("Nothing to create?");
> +
> +	for (i = start ; i < argc; i++) {

s/start ;/start;/ (everywhere).