Re: [PATCH v2] [GSoC] submodule--helper: introduce add-clone subcommand

[Shourya Shukla <shouryashukla.oo@gmail.com>]

Hi Atharva!

On Wed, Jun 2, 2021 at 6:43 PM Atharva Raykar <raykar.ath@gmail.com> wrote:
>
> Let's add a new "add-clone" subcommand to `git submodule--helper` with
> the goal of converting part of the shell code in git-submodule.sh
> related to `git submodule add` into C code. This new subcommand clones
> the repository that is to be added, and checks out to the appropriate
> branch.
>
> This is meant to be a faithful conversion that leaves the behaviour of
> 'submodule add' unchanged. The only minor change is that if a submodule name has
> been supplied with a name that clashes with a local submodule, the message shown
> to the user ("A git directory for 'foo' is found locally...") is prepended with
> "error" for clarity.

It would be better if commit messages are limited to 72 columns
(characters) per line.
Though you can obviously write longer lines on the list no problem.

> This is part of a series of changes that will result in all of 'submodule add'
> being converted to C.
>
> Signed-off-by: Atharva Raykar <raykar.ath@gmail.com>
> Mentored-by: Christian Couder <christian.couder@gmail.com>
> Mentored-by: Shourya Shukla <shouryashukla.oo@gmail.com>
> Based-on-patch-by: Shourya Shukla <shouryashukla.oo@gmail.com>
> Based-on-patch-by: Prathamesh Chavan <pc44800@gmail.com>


I and others before me used to sign off the previous authors using
'Signed-off-by:'. This trailer
has not been used yet so I am not sure if it should be used though I
prefer this over the former.
Maybe Christian could comment here?

> This is part of a series of changes that will result in all of 'submodule add'
> being converted to C, which is a more familiar language for Git developers, and
> paves the way to improve performance and portability.
>
> I have made this patch based on Shourya's patch[1]. I have decided to send the
> changes in smaller, more reviewable parts. The add-clone subcommand of
> submodule--helper is an intermediate change, while I work on translating all of
> the code.
>
> Another subcommand called 'add-config' will also be added in a separate patch
> that handles the configuration on adding the module.
>
> After those two changes look good enough, I will be converting whatever is left
> of 'git submodule add' in the git-submodule.sh past the flag parsing into C code
> by having one helper subcommand called 'git submodule--helper add' that will
> incorporate the functionality of the other two helpers, as well. In that patch,
> the 'add-clone' and 'add-config' subcommands will be removed from the commands
> array, as they will be called from within the C code itself.

Seems like a good approach! BTW, if this "extra" message is a bit long
like the one above, then
you can put it in a cover letter instead. If people really want to
read this extra information
they will read it in a cover letter as well.

Just supply the '--cover-letter' option when executing the 'git
format-patch' command.

> Changes since v1:
>  * Fixed typos, and made commit message more explicit
>  * Fixed incorrect usage string
>  * Some style changes were made

To save yourself the trouble of sieving the "top" or "noteworthy" changes from
the new version, you could instead just print the 'range-diff' between
the two versions.

You can do:
'git range-diff b1~n1..b1 b2~n2..b2'

Where:

- 'b1' is the first branch; 'n1' is the number of top commits you are
taking from 'b1' for
  comparison.

- 'b2' is the second branch; 'n2' is the number of top commits you are
taking from 'b2' for
  comparison.

It will print a very detailed output showing what differences were
there commit-wise
amongst the two branches. This can be put at the end of the cover
letter. Though, this
isn't necessary if your way seems better to you.

BTW, it would be helpful if you could send mails addressed to me on my
other email <periperidip@gmail.com>.