Re: [PATCH v2 1/2] promisor-remote: accept 0 as oid_nr in function

From: Junio C Hamano <gitster@pobox.com>
To: Jonathan Tan <jonathantanmy@google.com>
Cc: git@vger.kernel.org, stolee@gmail.com
Subject: Re: [PATCH v2 1/2] promisor-remote: accept 0 as oid_nr in function
Date: Thu, 02 Apr 2020 12:46:26 -0700	[thread overview]
Message-ID: <xmqqblo93c31.fsf@gitster.c.googlers.com> (raw)
In-Reply-To: <474eb27d9c136fb69e961546004cfb531d722e2c.1585854639.git.jonathantanmy@google.com> (Jonathan Tan's message of "Thu, 2 Apr 2020 12:19:16 -0700")

Jonathan Tan <jonathantanmy@google.com> writes:

> There are 3 callers to promisor_remote_get_direct() that first check if
> the number of objects to be fetched is equal to 0. Fold that check into
> promisor_remote_get_direct(), and in doing so, be explicit as to what
> promisor_remote_get_direct() does if oid_nr is 0 (it returns 0, success,
> immediately).

>
> Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
> ---
>  builtin/index-pack.c |  5 ++---
>  diff.c               | 11 +++++------
>  promisor-remote.c    |  3 +++
>  promisor-remote.h    |  8 ++++++++
>  unpack-trees.c       |  5 ++---
>  5 files changed, 20 insertions(+), 12 deletions(-)

Nice simplification.

> +/*
> + * Fetches all requested objects from all promisor remotes, trying them one at
> + * a time until all objects are fetched. Returns 0 upon success, and non-zero
> + * otherwise.

Good.

> + * If oid_nr is 0, this function returns 0 (success) immediately.

Is this worth saying?  If you ask to lazily grab 0 objects, it is
probably clear that no object would be read before the helper
returns.

When oid_nr==0 you are allowed to pass oids==NULL, but otherwise,
oids==NULL would be an error.  Is that the kind of difference you
wanted to point out, I wonder?

> + */
>  int promisor_remote_get_direct(struct repository *repo,
>  			       const struct object_id *oids,
>  			       int oid_nr);