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