From: Junio C Hamano <gitster@pobox•com>
To: Stefan Beller <sbeller@google•com>
Cc: bmwill@google•com, git@vger•kernel.org, pclouds@gmail•com
Subject: Re: [PATCHv5 0/5] submodule embedgitdirs
Date: Wed, 07 Dec 2016 14:35:16 -0800 [thread overview]
Message-ID: <xmqqzik7v04b.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20161207210157.18932-1-sbeller@google.com> (Stefan Beller's message of "Wed, 7 Dec 2016 13:01:52 -0800")
Stefan Beller <sbeller@google•com> writes:
> v5:
> * Add another layer of abstraction, i.e. the relocate_git_dir is only about
> moving a git dir of one repository. The submodule specific stuff (e.g.
> recursion into nested submodules) is in submodule.{c,h}
>
> This was motivated by reviews on the series of checkout aware of submodules
> building on top of this series, as we want to directly call the embed-git-dirs
> function without the overhead of spawning a child process.
OK. Comparing the last steps between this round and the previous
one, I do think the separation of the responsibility among helpers
is much more reasonable in this version, where:
- submodule_embed_git_dir() is given a single path and is
responsible for that submodule itself, which is done by calling
submodule_embed_git_dir_for_path() on itself, and its
sub-submodules, which is done by spawning the helper recursively
with appropriate super-prefix;
- submodule_embed_git_dir_for_path() computes where the given path
needs to be moved to using the knowledge specific to the
submodule subsystem, and asks relocate_gitdir() to perform the
actual relocation;
- relocate_gitdir() used to do quite a lot more, but now it is only
about moving an existing .git directory elsewhere and pointing to
the new location with .git file placed in the old location.
I would have called the second helper submodule_embed_one_git_dir(),
but that is a minor detail.
Very nicely done.
next prev parent reply other threads:[~2016-12-07 22:36 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-12-07 21:01 [PATCHv5 0/5] submodule embedgitdirs Stefan Beller
2016-12-07 21:01 ` [PATCHv5 1/5] submodule: use absolute path for computing relative path connecting Stefan Beller
2016-12-07 21:01 ` [PATCHv5 2/5] submodule helper: support super prefix Stefan Beller
2016-12-07 21:01 ` [PATCHv5 3/5] test-lib-functions.sh: teach test_commit -C <dir> Stefan Beller
2016-12-07 21:01 ` [PATCHv5 4/5] worktree: get worktrees from submodules Stefan Beller
2016-12-07 22:45 ` Junio C Hamano
2016-12-07 22:51 ` Stefan Beller
2016-12-08 1:14 ` Brandon Williams
2016-12-07 21:01 ` [PATCHv5 5/5] submodule: add embed-git-dir function Stefan Beller
2016-12-07 23:03 ` Junio C Hamano
2016-12-07 23:37 ` Stefan Beller
2016-12-08 1:09 ` Brandon Williams
2016-12-08 11:01 ` Duy Nguyen
2016-12-07 22:35 ` Junio C Hamano [this message]
2016-12-07 23:34 ` [PATCHv5 0/5] submodule embedgitdirs Junio C Hamano
2016-12-07 23:39 ` Stefan Beller
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=xmqqzik7v04b.fsf@gitster.mtv.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=bmwill@google$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=pclouds@gmail$(echo .)com \
--cc=sbeller@google$(echo .)com \
/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