Re: [PATCH v3 0/6] config.h

[Brandon Williams <bmwill@google.com>]

On 06/15, Junio C Hamano wrote:
> Brandon Williams <bmwill@google.com> writes:
> 
> > On 06/15, Junio C Hamano wrote:
> >
> >> ... so please eyeball the resulting 12 patches carefully when
> >> they are pushed out.
> >
> > Ugh, I'm terribly sorry.  Completely my bad as I didn't consider what
> > you would need to do on your end.  When I built my patches on top of his
> > I naively just applied his v4 to what ever the current origin/master was
> > at that point in time.  I'll be sure to be more careful with this
> > next time.
> 
> It's no big deal (otherwise I would have insisted you to rebase so
> that the end result can be merged to 'maint', instead of doing it
> myself).
> 
> But quite honestly, I do not understand why you rebased this on top
> of the alias thing.  Help me make sure that I correctly undertand
> what these two topics want to do:
> 
>  - The primary point of js/alias-early-config is to fix reading of
>    pager config from a wrong place when alias expansion is involved,
>    and its solution has a nice property that it simplifies the alias
>    lookup and avoids the unpleasant save/restore-env dance by
>    switching to use the early-config mechanism.
> 
>  - Unfortunately, the early-config mechanism is broken with respect
>    to multi-worktree configuration because it does not pay attention
>    to the common-dir stuff.
> 
>  - The primary objective of bw/config-h was to separate out the
>    configuration related things out of the kitchen-sink cache.h, but
>    as a nice side effect, it also fixes the early-config mechanism.
> 
> Assuming that the above reading of mine of these two topics are
> correct, we can conclude that even if we merge js/alias-early-config
> that forks from v2.13.0 to 'maint', the result by itself would
> regress the use of alias in multi-worktree configuration.  For it to
> be useful, it must be merged after bw/config-h gets merged.
> 
> So it looks to me that it would make more sense to build bw/config-h
> on v2.13.0 and then base js/alias-early-config on top of the result.
> If Dscho is too busy to rebase and you are volunteering to help,
> perhaps the right way to help would be for you to do that rebasing,
> not rebase bw/config-h on top of js/alias-early-config, which is
> backwards and does not buy us very much.  Of course, we could make
> the result of such rebasing into a single topic, but even in that
> case, the order of changes feel backwards if bw/config-h comes
> later.
> 
> Anyway, I think I have to tend to many more patches before I can
> push out today's integration result, so ...
> 
> 

Yeah I see that the order is probably backwards.  I think I just heard
Dscho say "I'm too busy" and went along with just rebasing on top of his
series.  If you do end up feeling like this should be done differently
(like if another reroll is needed etc) then I wouldn't mind taking
ownership and making the order more sane.

-- 
Brandon Williams