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