Re: [PATCH 0/2] pack-objects: freshen objects with multi-cruft packs

From: Taylor Blau <me@ttaylorr.com>
Date: Feb 27, 2025, 23:05 UTC
Message-ID: <Z8Dvr3F1t7JKuNQf@nand.local>
In-Reply-To: <CABPp-BEK3T28iqDFxbMHa7GVrythYs+ze7wCZ5Q-LDjOmquiVQ@mail.gmail.com>

On Thu, Feb 27, 2025 at 11:28:18AM -0800, Elijah Newren wrote:

Show 8 quoted lines

> > Previously the rule was to discard any object which appears in any
> > surviving pack, regardless of mtime. The rule now is to only discard an
> > object if it appears in either (a) a non-cruft pack which will survive
> > the repack, or (b) a cruft pack whose mtime for that object is older
> > than the one we are trying to pack.
>
> I think in (b) you got the meaning reversed, and instead mean s/older
> than/at least as new as/ ?

Oops, you are definitely right, that is a clear typo on my part.

> Code changes look good to me, but I had some wording suggestions in a
> few places for commit messages and comments.  (Sorry for missing some
> of those in my preliminary review before you sent this series to the
> list.)

No problem. I had only shared them before sending them here for a sanity check on the implementation, so I was figuring that I'd get some comments here on both the patch message and contents.

I adjusted the wording of both patches slightly to reflect your review. I'll hold off on sending another round for a day or so in case any other reviewers want to chime in. Thanks again for the careful review!

Thanks, Taylor