On 03/03, Nguyễn Thái Ngọc Duy wrote: > On Sat, Mar 3, 2018 at 9:54 AM, Duy Nguyen <pclouds@xxxxxxxxx> wrote: > > On Thu, Mar 1, 2018 at 2:09 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > >> Stefan Beller <sbeller@xxxxxxxxxx> writes: > >> > >>> On Wed, Feb 28, 2018 at 9:59 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > >>>> Duy Nguyen <pclouds@xxxxxxxxx> writes: > >>>> > >>>>> Looking at the full-series diff though, it makes me wonder if we > >>>>> should keep prepare_packed_git() private (i.e. how we initialize the > >>>>> object store, packfile included, is a private matter). How about > >>>>> something like this on top? > >>>> > >>>> Yup, that looks cleaner. > >>> > >>> I agree that it looks cleaner. So we plan on just putting > >>> it on top of that series? > >> > >> We tend to avoid "oops, that was wrong and here is a band aid on > >> top" for things that are still mushy, so it would be preferrable to > >> get it fixed inline, especially if there are more changes to the > >> other parts of the series coming. > > > > I agree with this. Stefan, if you're getting bored with rerolling > > refactor patches, I can update this series and send out v2. > > Since Stefan is traveling, I take this opportunity to reroll it. > Unfortunately, I think the fix should go in 46cd557bd9 (object-store: > move packed_git and packed_git_mru to object store - 2018-02-23) where > we start removing the global "packed_git". But that's in > sb/object-store, so.. I'm rerolling all three > > 01/44 - 05/44: nd/remove-ignore-env-field > > This series is moved up top. After this the patch that touch > alternate-db in sha1_file.c looks natural because no env is involved > anymore > > I also take this opportunity to introduce a new patch 01/44 to avoid > struct initialization that makes it hard to read and update. Later > patches are also simplified thanks to this. > > 06/44 - 32/44: sb/object-store > > 06/44 is updated to introduce raw_object_store_init() instead of > RAW_OBJECT_STORE_INIT macro. This function is now used to initialize > both main repo and submodule ones. > > 10/44 (moving "packed_git") also introduces two new access wrapper > get_packed_git() and get_packed_git_mru() > > 33/44 - 44/44: sb/packfiles-in-repository > > The only thing new here is 44/44 which makes prepare_packed_git() > internal. get_packed_git() and get_packed_git_mru() introduced > earlier will call prepare_packed_git() automatically. > > The whole thing is also available at > > https://github.com/pclouds/git/tree/ignore-env-object-store-packfiles > > And interdiff of all three, compared to what is currently in 'pu'. > Looks pretty good in my opinon: The only major issue I could find is with patch 40 and must be fixed before this can be merged. The rest of the series looks good. -- Brandon Williams
