From: Abhijeet Sonar Date: Tue, 25 Jun 2024 13:22:31 GMT Subject: Re: [PATCH v2] describe: refresh the index when 'broken' flag is used Message-ID: <68eb8651-c56e-449f-8102-b82506607387@gmail.com> In-Reply-To: Hi Karthik, thanks for taking the time to review my patch. On 25/06/24 17:57, Karthik Nayak wrote: > We should also call `strvec_clear(&update_index_cp.args);` to clear up > used memory. > > Nit: we could actually `cp` for both the child processes here. Will do those, thanks! > It would be nice to cleanup the repo at the end of the test by adding > `test_when_finished "rm -rf stat-dirty" &&` here All the files generated by the tests in this file are under `trash directory.t6120-describe/` which seems to be automatically deleted when all tests succeed.  Also, given that rest of the tests in this file don't perform any manual cleanup, I am not sure if doing so really falls under the scope of this patch. > You want to do everything apart from the repo init in a subshell, this > ensures we don't carry over the working directory to the next test. Ah, now I see why other tests wrap all commands except `git init` in parentheses.  Thanks! > Can't this be merged with the previous test? Having those separate looked nicer to me.  I will merge them together.