Hi Junio, On Tue, Dec 03, 2019 at 07:41:19AM -0800, Junio C Hamano wrote: > Denton Liu <liu.denton@xxxxxxxxx> writes: > > >> Especially if it is near the end of the series, just a single step > >> is OK. But is there anything that is glaringly wrong that needs a > >> reroll? Or would it be "this is good enough, so let's have them > >> cook in 'next' and graduate to 'master'---further clean-up can be > >> done after all the dust settles"? I have an impression that we > >> reached the latter by now. > > > > Perhaps the log message could use some improvement to document the > > discussion we had? I don't know if that's worth a reroll, though. Aside > > from that, I agree that it's ready for 'next'. > > 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 <liu.denton@xxxxxxxxx> The only change between this and the old commit message is the addition of the "The result of this..." paragraph. Thanks, Denton > > Thanks for working on this.
