On Fri, May 05, 2023 at 08:13:45PM -0400, Taylor Blau wrote: > On Fri, May 05, 2023 at 06:13:57PM -0400, Jeff King wrote: > > One thing that could make this a lot simpler is if the code was added to > > "are we recent" code paths in the first place. > > > > Something like this: > > This is quite nice, and I think that it's a good direction to push this > ~series~ patch in before queuing. That said, I wasn't sure about... > > > @@ -78,7 +144,7 @@ static void add_recent_object(const struct object_id *oid, > > struct object *obj; > > enum object_type type; > > > > - if (mtime <= data->timestamp) > > + if (!obj_is_recent(oid, mtime, data)) > > return; > > > > /* > > ...this hunk. That only kicks in if you have other recent object(s), > since the hooks are consulted as a side-effect of calling your new > `obj_is_recent()` function. > > I think in most cases that's fine, but if you had no otherwise-recent > objects around, then this code wouldn't kick in in the first place. > > I wonder if it might make more sense to call the hooks directly in > add_unseen_recent_objects_to_traversal(). OK, no, this is fine, but we'd need to make sure that want_recent_object() also understood the fake recent set here, since add_recent_loose() and add_recent_packed() both bail early when want_recent_object() returns 0. Thanks, Taylor
