From: Derrick Stolee <stolee@gmail.com>
Date: Wed, 24 Feb 2021 15:43:20 GMT
Subject: Re: [PATCH v2 05/10] diffcore-rename: compute dir_rename_counts in stages
Message-ID: <1c3b2aa7-d56f-00a6-cd19-0810d12540c0@gmail.com>
In-Reply-To: <3a29cf9e526fba0227a7eec92c0c6bd58a7850f0.1614123848.git.gitgitgadget@gmail.com>

On 2/23/2021 6:44 PM, Elijah Newren via GitGitGadget wrote:
> +	info->setup = 0;
> +	if (!dirs_removed)
> +		return;
>  	info->setup = 1;

This would probably be clearer as

	if (!dirs_removed) {
		info->setup = 0;
		return;
	}
	info->setup = 1;

> -MAYBE_UNUSED

Good cleanup.

> @@ -931,10 +974,17 @@ void diffcore_rename_extended(struct diff_options *options,
>  		remove_unneeded_paths_from_src(want_copies);
>  		trace2_region_leave("diff", "cull after exact", options->repo);
>  
> +		/* Preparation for basename-driven matching. */
> +		trace2_region_enter("diff", "dir rename setup", options->repo);
> +		initialize_dir_rename_info(&info,
> +					   dirs_removed, dir_rename_count);
> +		trace2_region_leave("diff", "dir rename setup", options->repo);
> +

The parts visible in this context are pretty trivial, but this
method _is_ doing a lot of work. Good to mark it with a trace
region so we can identify if/when it is a problem.

Thanks,
-Stolee