Re: [PATCH] submodule: stop sanitizing config options

[Stefan Beller <sbeller@google.com>]

On Thu, May 5, 2016 at 4:33 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Jeff King <peff@peff.net> writes:
>
>> I had originally thought after the final one that we could further clean
>> up by turning prepare_submodule_repo_env() into a static function. But
>> we can't; it gets called in one spot from submodule--helper. However,
>> while looking at it, I did notice that we probably want to squash this
>> into the final patch (since sanitize_submodule_config went away
>> completely):
>>
>> diff --git a/submodule.h b/submodule.h
>> index 48690b1..869d259 100644
>> --- a/submodule.h
>> +++ b/submodule.h
>> @@ -43,19 +43,10 @@ int find_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_nam
>>  int push_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_name);
>>  void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);
>>
>> -/*
>> - * This function is intended as a callback for use with
>> - * git_config_from_parameters(). It ignores any config options which
>> - * are not suitable for passing along to a submodule, and accumulates the rest
>> - * in "data", which must be a pointer to a strbuf. The end result can
>> - * be put into $GIT_CONFIG_PARAMETERS for passing to a sub-process.
>> - */
>> -int sanitize_submodule_config(const char *var, const char *value, void *data);
>> -
>>  /*
>>   * Prepare the "env_array" parameter of a "struct child_process" for executing
>>   * a submodule by clearing any repo-specific envirionment variables, but
>> - * retaining any config approved by sanitize_submodule_config().
>> + * retaining any config in the environment.
>>   */
>>  void prepare_submodule_repo_env(struct argv_array *out);
>>
>>
>> -Peff
>
> Hmph, Stefan, do you want to keep this (if you want to resurrect the
> function in some future, for example)?

IMHO it is easier to revert or rewrite than to carry unused code?
Unused code is not tested and untested code is broken code.
And relying on broken code in the future will guarantee bugs.
(Sure new code may also have bugs, but I just think that bugs
in newly written code can be fixed easier)

I would prefer to get rid of it, i.e. taking that patch.

Thanks,
Stefan