| Message ID | 6957e54f51759ff1b6d2469bc40c9b966635595d.1681764848.git.me@ttaylorr.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | gc: enable cruft packs by default | expand |
On Mon, Apr 17, 2023 at 04:54:33PM -0400, Taylor Blau wrote: > In the last commit, we refactored some of the tests in t6500 to make > clearer when cruft packs will and won't be generated by `git gc`. > > Add the remaining cases not covered by the previous patch into this one, > which enumerates all possible combinations of arguments that will > produce (or not produce) a cruft pack. > > This prepares us for the following commit, which will change the default > of `gc.cruftPacks` by ensuring that we understand which invocations do > and do not change as a result. I think "the following commit" is really patch 9. Patch 8 adjusts t6501 (and likewise says "like the previous commits"). It would probably make more sense to put patch 8 before patches 6 and 7. Probably not worth a re-roll on its own, but if you're doing one anyway, it should be easy to do. -Peff
On Tue, Apr 18, 2023 at 06:48:13AM -0400, Jeff King wrote: > On Mon, Apr 17, 2023 at 04:54:33PM -0400, Taylor Blau wrote: > > > In the last commit, we refactored some of the tests in t6500 to make > > clearer when cruft packs will and won't be generated by `git gc`. > > > > Add the remaining cases not covered by the previous patch into this one, > > which enumerates all possible combinations of arguments that will > > produce (or not produce) a cruft pack. > > > > This prepares us for the following commit, which will change the default > > of `gc.cruftPacks` by ensuring that we understand which invocations do > > and do not change as a result. > > I think "the following commit" is really patch 9. Patch 8 adjusts t6501 > (and likewise says "like the previous commits"). It would probably make > more sense to put patch 8 before patches 6 and 7. I'm glad you said something, because shortly after I sent this series I was annoyed with myself for not organizing the preparatory patches in order of their test number. I reordered them like so: c02f80e1e2 t/t5304-prune.sh: prepare for `gc --cruft` by default 2e0fb1382b t/t6501-freshen-objects.sh: prepare for `gc --cruft` by default c916e6d356 t/t6500-gc.sh: refactor cruft pack tests f9f7b2137b t/t6500-gc.sh: add additional test cases 5a74520b35 t/t9300-fast-import.sh: prepare for `gc --cruft` by default Which feels much better to me. > Probably not worth a re-roll on its own, but if you're doing one anyway, > it should be easy to do. Agreed on both. I'm rerolling a few tiny things anyways, so I'll squash this in, too. Thanks, Taylor
diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh index a2f988c5c2..e7d3d1448f 100755 --- a/t/t6500-gc.sh +++ b/t/t6500-gc.sh @@ -218,6 +218,7 @@ assert_no_cruft_packs () { for argv in \ "gc --cruft" \ "-c gc.cruftPacks=true gc" \ + "-c gc.cruftPacks=false gc --cruft" \ "-c feature.experimental=true gc" \ "-c gc.cruftPacks=true -c feature.experimental=false gc" do @@ -243,6 +244,9 @@ do done for argv in \ + "gc --no-cruft" \ + "-c gc.cruftPacks=false gc" \ + "-c gc.cruftPacks=true gc --no-cruft" \ "-c feature.expiremental=true -c gc.cruftPacks=false gc" \ "-c feature.experimental=false gc" do
In the last commit, we refactored some of the tests in t6500 to make clearer when cruft packs will and won't be generated by `git gc`. Add the remaining cases not covered by the previous patch into this one, which enumerates all possible combinations of arguments that will produce (or not produce) a cruft pack. This prepares us for the following commit, which will change the default of `gc.cruftPacks` by ensuring that we understand which invocations do and do not change as a result. Signed-off-by: Taylor Blau <me@ttaylorr.com> --- t/t6500-gc.sh | 4 ++++ 1 file changed, 4 insertions(+)