Re: [PATCH 35/35] submodule: convert push_unpushed_submodules to take a struct refspec

From: Stefan Beller <sbeller@google.com>
Date: May 15, 2018, 16:52 UTC
Message-ID: <CAGZ79kaOt4oonsEMn47Rp_xL4_tGy6k78kFZFfxUOg_4sD_xtg@mail.gmail.com>
In-Reply-To: <87k1s52va3.fsf@evledraar.gmail.com>

On Tue, May 15, 2018 at 1:11 AM, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:

Show 24 quoted lines

>
> On Mon, May 14 2018, Brandon Williams wrote:
>
>> Convert 'push_unpushed_submodules()' to take a 'struct refspec' as a
>> parameter instead of an array of 'const char *'.
>> [...]
>> diff --git a/submodule.h b/submodule.h
>> index e5526f6aa..aae0c9c8f 100644
>> --- a/submodule.h
>> +++ b/submodule.h
>> @@ -100,9 +100,10 @@ extern int submodule_touches_in_range(struct object_id *a,
>>  extern int find_unpushed_submodules(struct oid_array *commits,
>>                                   const char *remotes_name,
>>                                   struct string_list *needs_pushing);
>> +struct refspec;
>>  extern int push_unpushed_submodules(struct oid_array *commits,
>>                                   const struct remote *remote,
>> -                                 const char **refspec, int refspec_nr,
>> +                                 const struct refspec *rs,
>>                                   const struct string_list *push_options,
>>                                   int dry_run);
>>  /*
>
> Why do you prefer doing this to having this on top?:

The fewer includes in header files the better, as then the headers themselves don't have dependencies. (Otherwise we'd end up multiplying cache.h ;)

In the source files we have to include all needed headers, but for the headers, it is better if we can just get away with declaring the existence of a struct.

This way we reduce compile time, so I'd am not keen on your patch on top.

This is discussed a lot on stackoverflow, e.g.: https://softwareengineering.stackexchange.com/questions/195806/forward-declaration-vs-include https://stackoverflow.com/a/15828094

Stefan