Subject: Re: [PATCH 00/44] reroll nd/remove-ignore-env.. sb/object-store and sb/packfiles..

From: Brandon Williams <[email-masked]>
Date: Wed, 21 Mar 2018 15:43:50 -0700

On 03/03, Nguyễn Thái Ngọc Duy wrote:
> On Sat, Mar 3, 2018 at 9:54 AM, Duy Nguyen <[email-masked]> wrote:
> > On Thu, Mar 1, 2018 at 2:09 AM, Junio C Hamano <[email-masked]> wrote:
> >> Stefan Beller <[email-masked]> writes:
> >>
> >>> On Wed, Feb 28, 2018 at 9:59 AM, Junio C Hamano <[email-masked]> wrote:
> >>>> Duy Nguyen <[email-masked]> 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