On Wed, Aug 17, 2016 at 3:12 PM, Junio C Hamano <gits...@pobox.com> wrote:
> Stefan Beller <sbel...@google.com> writes:
>>  Added a default for alternateErrorStrategy and hence fixed the null pointer
>>  for error_strategy in prepare_possible_alternates(),
> Looks much better.
>> +submodule.alternateLocation::
>> +     Specifies how the submodules obtain alternates when submodules are
>> +     cloned. Possible values are `no`, `superproject`.
>> +     By default `no` is assumed, which doesn't add references. When the
>> +     value is set to `superproject` the submodule to be cloned computes
>> +     its alternates location relative to the superprojects alternate.
> I am not seeing a code that handles 'no' and any other string that
> is not 'superproject' differently, though.
> I can see that "clone" has codepath that writes 'superproject' to
> the variable, but the only thing that seems to care what value the
> variable is set to checks "no".  When somebody sets the variable to
> "yes", shouldn't we at least say "Sorry, I do not understand" and
> preferrably stop before spreading potential damage?  We'd surely end
> up doing something that the user who set the value to "yes" did not
> expect.
> There is something still missing?

>> +static void prepare_possible_alternates(const char *sm_name,
>> +             struct string_list *reference)
>> +{
>> +     sas.submodule_name = sm_name;
>> +     sas.reference = reference;
>> +     if (!strcmp(error_strategy, "die"))
>> +             sas.error_mode = SUBMODULE_ALTERNATE_ERROR_DIE;
>> +     if (!strcmp(error_strategy, "info"))
>> +             sas.error_mode = SUBMODULE_ALTERNATE_ERROR_INFO;

(see below first)
Here goes the same for alternateErrorStrategy

>> +     if (!strcmp(sm_alternate, "superproject"))
>> +             foreach_alt_odb(add_possible_reference_from_superproject, 
>> &sas);

here is where we would add

    else if (!strcmp(sm_alternate, "no")
        ; /* do nothing */
        die(_("What's wrong with you?"));

Initially I did not add that as I considered it not relevant. But I
guess it helps as a typo checker as well and it is more comforting
if a wrong value yields an error. Also it is consistent with the rest of

Thanks again,
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to