I uploaded a new patch; a few comments inline below... On Wed, Feb 12, 2014 at 1:19 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > szager@xxxxxxxxxxxx writes: > > Also I'd suggest s/pack_data_fn/collect_pack_data/ or something. > "_fn" may be a good suffix for typedef'ed typename used in a > callback function, but for a concrete function, it only adds noise > without giving us anything new. Fixed this everywhere. >> static struct cached_object *find_cached_object(const unsigned char *sha1) >> @@ -468,7 +469,6 @@ static unsigned int pack_open_fds; >> static unsigned int pack_max_fds; >> static size_t peak_pack_mapped; >> static size_t pack_mapped; >> -struct packed_git *packed_git; > > Hmm, any particular reason why only this variable and not others are > moved up? No, just need packed_git declared before use. I moved all the static variables up, for clarity. >> + foreach_packed_git(find_pack_fn, NULL, &fpd); >> + if (fpd.found_pack && !exclude && >> + (incremental || >> + (local && fpd.found_non_local_pack) || >> + (ignore_packed_keep && fpd.found_pack_keep))) >> + return 0; > > When told to do --incremental, we used to return 0 from this > function immediately once we find the object in one pack, without > going thru the list of packs. Now we let foreach to loop thru all > of them and then return 0. Does this difference matter? A similar > difference may exist for local/keep but I did not think it through. Fixed. Thanks, Stefan -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html