From: Jonathan Tan <jonathantanmy@google.com>
Date: Thu, 15 Nov 2018 19:54:16 GMT
Subject: Re: [PATCH 18/23] submodule: use submodule repos for object lookup
Message-ID: <20181115195416.21890-1-jonathantanmy@google.com>
In-Reply-To: <20181114001306.138053-19-sbeller@google.com>

> +/*
> + * Initialize 'out' based on the provided submodule path.
> + *
> + * Unlike repo_submodule_init, this tolerates submodules not present
> + * in .gitmodules. This function exists only to preserve historical behavior,
> + *
> + * Returns 0 on success, -1 when the submodule is not present.
>   */
> -static void show_submodule_header(struct diff_options *o, const char *path,
> +static struct repository *open_submodule(const char *path)

The function documentation needs to be reworded - there's no "out", and
the return value is now a possibly NULL pointer to struct repository.

> +{
> +	struct strbuf sb = STRBUF_INIT;
> +	struct repository *out = xmalloc(sizeof(*out));
> +
> +	if (submodule_to_gitdir(&sb, path) || repo_init(out, sb.buf, NULL)) {
> +		strbuf_release(&sb);
> +		free(out);
> +		return NULL;
> +	}
> +
> +	out->submodule_prefix = xstrdup(path);

I've discussed this submodule_prefix line before [1] - do we really need
this? Tests pass even if I remove this line.

Other than that, this patch looks good.

[1] https://public-inbox.org/git/20181019203750.110741-1-jonathantanmy@google.com/