public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox•com>
To: Brandon Williams <bmwill@google•com>
Cc: git@vger•kernel.org, peff@peff•net, jrnieder@gmail•com,
	Johannes.Schindelin@gmx•de
Subject: Re: [PATCH v3 0/6] config.h
Date: Thu, 15 Jun 2017 14:09:09 -0700	[thread overview]
Message-ID: <xmqqzid9yn4a.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20170615203332.GB176947@google.com> (Brandon Williams's message of "Thu, 15 Jun 2017 13:33:32 -0700")

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 ...



  reply	other threads:[~2017-06-15 21:09 UTC|newest]

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-12 21:34 [PATCH 0/4] config.h Brandon Williams
2017-06-12 21:34 ` [PATCH 1/4] config: create config.h Brandon Williams
2017-06-12 21:34 ` [PATCH 2/4] config: remove git_config_iter Brandon Williams
2017-06-13  0:49   ` Jonathan Nieder
2017-06-13  0:57     ` Jeff King
2017-06-12 21:34 ` [PATCH 3/4] config: don't include config.h by default Brandon Williams
2017-06-12 21:34 ` [PATCH 4/4] config: don't implicitly use gitdir Brandon Williams
2017-06-13  1:05   ` Jonathan Nieder
2017-06-13  1:23     ` Brandon Williams
2017-06-13  1:33       ` Jonathan Nieder
2017-06-13  1:38       ` Jonathan Nieder
2017-06-13  2:59         ` Jeff King
2017-06-13  6:16           ` Brandon Williams
2017-06-13  6:45             ` Jeff King
2017-06-13  7:08             ` Jeff King
2017-06-13 14:43               ` Brandon Williams
2017-06-13 17:06           ` Jonathan Nieder
2017-06-13  5:52         ` Brandon Williams
2017-06-13  6:29           ` Jeff King
2017-06-13 14:47             ` Brandon Williams
2017-06-12 21:45 ` [PATCH 0/4] config.h Jeff King
2017-06-12 21:53   ` Brandon Williams
2017-06-12 22:02     ` Jeff King
2017-06-12 22:06       ` Brandon Williams
2017-06-13  1:07 ` Jonathan Nieder
2017-06-13 21:03 ` [PATCH v2 0/6] config.h Brandon Williams
2017-06-13 21:03   ` [PATCH v2 1/6] config: create config.h Brandon Williams
2017-06-13 21:13     ` Jonathan Nieder
2017-06-13 21:03   ` [PATCH v2 2/6] config: remove git_config_iter Brandon Williams
2017-06-13 21:14     ` Jonathan Nieder
2017-06-13 21:03   ` [PATCH v2 3/6] config: don't include config.h by default Brandon Williams
2017-06-13 21:58     ` Jonathan Nieder
2017-06-13 21:03   ` [PATCH v2 4/6] config: don't implicitly use gitdir Brandon Williams
2017-06-13 21:08     ` Jonathan Nieder
2017-06-13 21:38       ` Brandon Williams
2017-06-13 21:51         ` Jonathan Nieder
2017-06-13 21:55           ` Junio C Hamano
2017-06-13 22:05             ` Jonathan Nieder
2017-06-14  4:40               ` Jacob Keller
2017-06-14  6:25         ` Jeff King
2017-06-14 17:14           ` Brandon Williams
2017-06-13 21:03   ` [PATCH v2 5/6] setup: teach discover_git_directory to respect the commondir Brandon Williams
2017-06-14  6:15     ` Jeff King
2017-06-14 17:19       ` Brandon Williams
2017-06-13 21:03   ` [PATCH v2 6/6] config: respect commondir Brandon Williams
2017-06-14 18:07   ` [PATCH v3 0/6] config.h Brandon Williams
2017-06-14 18:07     ` [PATCH v3 1/6] config: create config.h Brandon Williams
2017-06-14 18:07     ` [PATCH v3 2/6] config: remove git_config_iter Brandon Williams
2017-06-14 18:07     ` [PATCH v3 3/6] config: don't include config.h by default Brandon Williams
2017-06-14 18:07     ` [PATCH v3 4/6] setup: teach discover_git_directory to respect the commondir Brandon Williams
2017-06-14 18:07     ` [PATCH v3 5/6] config: respect commondir Brandon Williams
2017-06-14 18:07     ` [PATCH v3 6/6] config: don't implicitly use gitdir or commondir Brandon Williams
2017-06-15 19:59     ` [PATCH v3 0/6] config.h Junio C Hamano
2017-06-15 20:33       ` Brandon Williams
2017-06-15 21:09         ` Junio C Hamano [this message]
2017-06-15 21:18           ` Brandon Williams
2017-06-16  0:12             ` Brandon Williams

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqzid9yn4a.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox$(echo .)com \
    --cc=Johannes.Schindelin@gmx$(echo .)de \
    --cc=bmwill@google$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=jrnieder@gmail$(echo .)com \
    --cc=peff@peff$(echo .)net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox