Web lists-archives.com

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




On Tue, May 15, 2018 at 1:11 AM, Ævar Arnfjörð Bjarmason
<avarab@xxxxxxxxx> wrote:
>
> 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