Subject: Re: [PATCH v5 22/26] t7700: consolidate code into test_no_missing_in_packs()

From: Junio C Hamano <[email-masked]>
Date: Wed, 04 Dec 2019 10:13:57 -0800

Denton Liu <[email-masked]> writes:

>> Sure, let's see what you have in mind.
>
> Here's a complete replacement for the commit message:
>
> 	t7700: consolidate code into test_no_missing_in_packs()
>
> 	The code to test that objects were not missing from the packfile was
> 	duplicated many times. Extract the duplicated code into
> 	test_no_missing_in_packs() and use that instead.
>
> 	Refactor the resulting extraction so that if any git commands fail,
> 	their return codes are not silently lost.
>
> 	We were using sed to filter lines. Although not incorrect, this is
> 	exactly what grep is built for. Replace this invocation of sed with grep
> 	so that we use the correct tool for the job.
>
> 	Instead of verifying each file of `alt_objects/pack/*.idx` individually
> 	in a for-loop, batch them together into one verification step.
>
> 	The original testing construct was O(n^2): it used a grep in a loop to
> 	test whether any objects were missing in the packfile. Rewrite this to
> 	sort the files then use `comm -23` so that finding missing lines from
> 	the original file is done more efficiently.
>
> 	The result of this is that we end up with a `grep | cut | sort`
> 	pipeline. Previously, we were extracting the `sha1` as part of the
> 	`while read sha1 rest` loop. Since we removed the while-loop, we need to
> 	use `cut` to extract the `sha1` field. Note that we could have chosen to
> 	combine the `grep | cut` into a single `sed` invocation but we
> 	consciously leave it separate as it makes the intent more clear.
>
> 	While we're at it, add a space to `commit_and_pack ()` for style.
>
> 	Signed-off-by: Denton Liu <[email-masked]>
>
> The only change between this and the old commit message is the addition
> of the "The result of this..." paragraph.

Ah, you were planning to only update the log message?  Then let's
not bother.  I do not think we would want to encourage "grep piped
to cut" as a good pattern for others to follow and a single sed that
finds the relevant lines and munges the content of these lines into
what is desired conveys the intent clearly and more concisely (I
was hoping that that was what you had in mind for a reroll).