Re: [PATCH] push: do not add submodule odb as an alternate when recursing on demand

[Jonathan Nieder <jrnieder@gmail.com>]

Stefan Beller wrote:

> Use is_submodule_populated_gently instead, which is simpler and
> cheaper.
[...]
> --- a/submodule.c
> +++ b/submodule.c
> @@ -966,7 +966,9 @@ static int push_submodule(const char *path,
>  			  const struct string_list *push_options,
>  			  int dry_run)
>  {
> -	if (add_submodule_odb(path))
> +	int code;
> +
> +	if (!is_submodule_populated_gently(path, &code))
>  		return 1;

Ah, I forgot about this detail.  I don't think it should block this
patch (so my Reviewed-by still stands), but I wonder why this needs to
be gentle.  add_submodule_odb is gentle so that is the conservative
thing to do, but that doesn't mean it is the *right* thing to do.

If this passed NULL instead of &code as the second argument, would
anything break?

Could there be a comment explaining what kind of error we are
expecting and why it is okay to continue when that error is
encountered without any error handling?

Thanks,
Jonathan