From: Jonathan Tan <jonathantanmy@google.com>
Date: Wed, 21 Jun 2017 22:48:37 GMT
Subject: Re: [PATCH v3 20/20] ls-files: use repository object
Message-ID: <20170621154837.566b67ac@twelve2.svl.corp.google.com>
In-Reply-To: <20170620191951.84791-21-bmwill@google.com>

On Tue, 20 Jun 2017 12:19:51 -0700
Brandon Williams <bmwill@google.com> wrote:

> -static void show_ce_entry(const struct index_state *istate,
> -			  const char *tag, const struct cache_entry *ce)
> +static void show_ce(struct repository *repo, struct dir_struct *dir,
> +		    const struct cache_entry *ce, const char *fullname,
> +		    const char *tag)

This function is getting complicated - in particular, it's getting a
"fullname" which is, in a sense, redundant with "repo" and "ce" (but
that seems necessary for efficiency). Maybe add a comment describing the
function?

[snip]

> @@ -651,8 +610,9 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
>  	if (require_work_tree && !is_inside_work_tree())
>  		setup_work_tree();
>  
> -	if (recurse_submodules)
> -		compile_submodule_options(argv, &dir, show_tag);
> +	if (recurse_submodules) {
> +		repo_read_gitmodules(the_repository);
> +	}

No need for braces.