From: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
Date: Sun, 08 Aug 2021 18:01:00 GMT
Subject: Re: [GSoC] [PATCH v4 0/8] submodule: convert the rest of 'add' to C
Message-ID: <1131ec9a-d836-b45a-63c8-6f1d39dd3399@gmail.com>
In-Reply-To: <20210807071613.99610-1-raykar.ath@gmail.com>

On 07/08/21 12:46 pm, Atharva Raykar wrote:
> Changes since v3:
> 
> * Rename the libified dir helper and update the docstring.
>

Just a note. I'm not really sure about this yet, the fact
that this series depends on the change introduced by
'ar/submodule-add-config'[1] might be worth mentioning in
re-rolls too. This could help the maintainer to easily identify
the topic dependency :-)

[1]: https://lore.kernel.org/git/20210801063352.50813-1-raykar.ath@gmail.com/

And ...

> Atharva Raykar (8):
>    submodule--helper: add options for compute_submodule_clone_url()
>    submodule--helper: refactor resolve_relative_url() helper
>    submodule--helper: remove repeated code in sync_submodule()
>    dir: libify and export helper functions from clone.c
>    submodule--helper: convert the bulk of cmd_add() to C
>    submodule--helper: remove add-clone subcommand
>    submodule--helper: remove add-config subcommand
>    submodule--helper: remove resolve-relative-url subcommand
> 
>   builtin/clone.c             | 118 +-------------
>   builtin/submodule--helper.c | 307 +++++++++++++++++++-----------------
>   dir.c                       | 114 +++++++++++++
>   dir.h                       |  10 ++
>   git-submodule.sh            |  96 +----------
>   5 files changed, 290 insertions(+), 355 deletions(-)
> 
> Range-diff against v3:

Thanks for consistently including a useful range-diff!

-- 
Sivaraam

> -:  ---------- > 1:  75edf24186 submodule--helper: add options for compute_submodule_clone_url()
> -:  ---------- > 2:  8e7a3e727a submodule--helper: refactor resolve_relative_url() helper
> -:  ---------- > 3:  82961ddd02 submodule--helper: remove repeated code in sync_submodule()
> 1:  99d139375d ! 4:  fa97d6801e dir: libify and export helper functions from clone.c
>      @@ builtin/clone.c: int cmd_clone(int argc, const char **argv, const char *prefix)
>        	else
>       -		dir = guess_dir_name(repo_name, is_bundle, option_bare);
>       -	strip_trailing_slashes(dir);
>      -+		dir = guess_target_dir_from_git_url(repo_name, is_bundle, option_bare);
>      ++		dir = git_url_basename(repo_name, is_bundle, option_bare);
>       +	strip_dir_trailing_slashes(dir);
>        
>        	dest_exists = path_exists(dir);
>      @@ dir.c: int is_empty_dir(const char *path)
>        	return ret;
>        }
>        
>      -+char *guess_target_dir_from_git_url(const char *repo, int is_bundle, int is_bare)
>      ++char *git_url_basename(const char *repo, int is_bundle, int is_bare)
>       +{
>       +	const char *end = repo + strlen(repo), *start, *ptr;
>       +	size_t len;
>      @@ dir.h: static inline int is_dot_or_dotdot(const char *name)
>        int is_empty_dir(const char *dir);
>        
>       +/*
>      -+ * Retrieve a target directory name by reading "humanish" part of the
>      -+ * given Git URL.
>      ++ * Retrieve the "humanish" basename of the given Git URL.
>       + *
>       + * For example:
>       + * 	/path/to/repo.git => "repo"
>       + * 	host.xz.foo/.git => "foo"
>       + */
>      -+char *guess_target_dir_from_git_url(const char *repo, int is_bundle, int is_bare);
>      ++char *git_url_basename(const char *repo, int is_bundle, int is_bare);
>       +void strip_dir_trailing_slashes(char *dir);
>       +
>        void setup_standard_excludes(struct dir_struct *dir);
> 2:  11eea777ba ! 5:  a3aa25518d submodule--helper: convert the bulk of cmd_add() to C
>      @@ builtin/submodule--helper.c: static int add_config(int argc, const char **argv,
>       +
>       +	add_data.repo = argv[0];
>       +	if (argc == 1)
>      -+		add_data.sm_path = guess_target_dir_from_git_url(add_data.repo, 0, 0);
>      ++		add_data.sm_path = git_url_basename(add_data.repo, 0, 0);
>       +	else
>       +		add_data.sm_path = xstrdup(argv[1]);
>       +
> 3:  51393cd99b = 6:  9667159d4b submodule--helper: remove add-clone subcommand
> 4:  50cedcd8a8 = 7:  dc87b5627a submodule--helper: remove add-config subcommand
> 5:  02558da532 = 8:  ea08e4fbad submodule--helper: remove resolve-relative-url subcommand
>