Re: [PATCH 13/16] submodule: teach unpack_trees() to update submodules

[Brandon Williams]

On 11/15, Stefan Beller wrote:
> +             if (!S_ISGITLINK(ce->ce_mode)) {
> +                     int flags = 
> CE_MATCH_IGNORE_VALID|CE_MATCH_IGNORE_SKIP_WORKTREE;

For readability you may want to have spaces between the two flags

> +     if (o->index_only
> +         || (!((old->ce_flags & CE_VALID) || ce_skip_worktree(old))
> +             && (o->reset || ce_uptodate(old))))
> +             return 0;

The coding guidelines say that git prefers to have the logical operators
on the right side like this:

if (o->index_only ||
    (!((old->ce_flags & CE_VALID) || ce_skip_worktree(old))     &&
     (o->reset || ce_uptodate(old))))
        return 0;

It also says the other way is ok too, just a thought :)

> +                             if (submodule_is_interesting(old->name, 
> null_sha1)
> +                                 && is_submodule_checkout_safe(new->name, 
> &new->oid))
> +                                     return 0;

here too.

> +                     } else {
> +                             /*
> +                              * new is not a submodule any more, so only
> +                              * care if we care:
> +                              */
> +                             if (submodule_is_interesting(old->name, 
> null_sha1)
> +                                 && ok_to_remove_submodule(old->name))
> +                                     return 0;

and here

-- 
Brandon Williams