Subject: Re: [PATCHv5 0/5] submodule embedgitdirs

From: Junio C Hamano <[email-masked]>
Date: Wed, 07 Dec 2016 14:35:16 -0800

Stefan Beller <[email-masked]> 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.