Re: [PATCH v2 1/3] serialize collection of changed submodules

From: Heiko Voigt <hvoigt@hvoigt.net>
Date: Oct 12, 2016, 13:11 UTC
Message-ID: <20161012131149.GC84247@book.hvoigt.net>
In-Reply-To: <CAGZ79kZiY56-84aThH1F02E_HzCTAK3KSYLbyP1D5GUAt892cw@mail.gmail.com>

On Fri, Oct 07, 2016 at 10:59:29AM -0700, Stefan Beller wrote:

Show 9 quoted lines

> On Fri, Oct 7, 2016 at 8:06 AM, Heiko Voigt <hvoigt@hvoigt.net> wrote:
> > +static void free_submodules_sha1s(struct string_list *submodules)
> > +{
> > +       int i;
> > +       for (i = 0; i < submodules->nr; i++) {
> > +               struct string_list_item *item = &submodules->items[i];
> 
> You do not seem to make use of `i` explicitely, so
> for_each_string_list_item might be more readable here?

Will change.

Show 16 quoted lines

> > @@ -603,12 +645,23 @@ int find_unpushed_submodules(unsigned char new_sha1[20],
> >                 die("revision walk setup failed");
> >
> >         while ((commit = get_revision(&rev)) != NULL)
> > -               find_unpushed_submodule_commits(commit, needs_pushing);
> > +               find_unpushed_submodule_commits(commit, &submodules);
> >
> >         reset_revision_walk();
> >         free(sha1_copy);
> >         strbuf_release(&remotes_arg);
> >
> > +       for (i = 0; i < submodules.nr; i++) {
> > +               struct string_list_item *item = &submodules.items[i];
> 
> You do not seem to make use of `i` explicitely, so
> for_each_string_list_item might be more readable here?

As above.

Cheers Heiko