On Tue, Nov 15, 2016 at 4:14 PM, David Turner <David.Turner@xxxxxxxxxxxx> wrote:
>> +int submodule_is_interesting(const char *path, const unsigned char
>> +*sha1) {
>
> This is apparently only ever (in this series) called with null_sha1. So either this arg is unnecessary, or there are bugs elsewhere in the code.
I was torn when writing the series, as I initially had submodule_is_interesting
with no sha1 argument and it turned out to be buggy in my first
initial implementation,
which lead me to thinking the sha1 actually matters.
The line of thinking was similar to loading the submodules from the
submodule-config cache as that also has different values for different sha1s,
e.g. a submodule is only interesting if submodule.<name>.update != none,
which can have changed with different sha1s.
I refactored the series since then to call the _is_initeresting method
at different times
(before and after the actual checkout), such that we implicitly have
the correct sha1
while calling it.
So I would argue the sha1 argument is not needed. I'll remove it.