git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Chinmoy Chakraborty <chinmoy12c@gmail.com>
To: Christian Couder <christian.couder@gmail.com>, git@vger.kernel.org
Subject: Re: [GSoC] Draft Proposal (Convert submodule to builtin)
Date: Wed, 7 Apr 2021 01:23:54 +0530	[thread overview]
Message-ID: <f58da7ee-1f35-0e59-a5ac-ae1a268719ed@gmail.com> (raw)
In-Reply-To: <CAP8UFD0RyhnurYdWt1xWF2O-bpk-irSs71+XH1Zd8ghdzRKZ8Q@mail.gmail.com>


On 4/5/21 8:14 PM, Christian Couder wrote:
> You could add something like "so that the `git submodule` is fully
> implemented using C".
>
Sure I'll add this.


>> ##Work to be done##
>>
>> The only command that is left to be ported is `git submodule add`.
>> The previous work on this by Shourya Shukla in GSoC 2020, did
>> not reach a successful merge due to some issues in design and
>> was kicked out because it has been stale for so long.
> Maybe you could explain a bit what "kicked out" means, or replace it
> with something more explicit, for people who don't know well how Git
> development works.

Okay, I'll explain it in simple terms. Actually in the docs version

I added a link to the `kicked out` phrase directing to :

https://github.com/git/git/blob/1861aa482a38ae84f597cede48167ab43e7e50a3/whats-cooking.txt#L1158-L1169

I'll add this in the textual format too.


>> The first
>> and foremost aim of the project will be to finish porting this
>> command.
> You mean the "add" sub-command, or the full "submodule" command?

The "add" command.


> Before porting the `git submodule add` command the initial work
>> would be dedicated to the implementation of small helper functions
>> in the form of small patches, which would be directly used by the
>> `add` command. This workflow is based on the suggestion by
>> Junio C Hamano on the thread:
>> https://lore.kernel.org/git/xmqqd01sugrg.fsf@gitster.c.googlers.com/.
>>
>> This workflow would help in the following ways:
>>
>> - It would help in sending patches in a small digestible format.
>> - It would help the reviewers easily review those small units
>>     of patches in a single sitting.
>> - It would help keep small logical units of code in different clean commits.
> Yeah, nice!
>
> How does this compare with Shourya's work? Will this avoid the design
> issues in Shourya's work?

It would help in faster and more thorough reviews by the reviewers.

Since last time, the patches sent were quite large in size, it became quite

tedious and difficult to review the patch at once. Due to this, the review

and changes cycle slacked off and hence the patch never made it to master.

Sending the patches in small chunks and separate logic units would speed up

the review cycle.


> Nice! Maybe you could give an example and tell:
> - how you would name one of the above function,
> - what would be its arguments,
> - perhaps how you would test it
> ...

Sure I'll add example prototype of the functions.


>> - After implementation of all these helper methods, the main
>>     `module_add()` function would be implemented using the helper
>>     functions listed above as well as those helper functions which
>>     are predefined.
>> =================================================
> Ok, I will review the other parts later.
>
> Thanks!
Thanks for the review, really helpful :).

  reply	other threads:[~2021-04-06 19:52 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-01 11:40 [GSoC] Draft Proposal (Convert submodule to builtin) Chinmoy Chakraborty
2021-04-02  5:32 ` Bagas Sanjaya
2021-04-02  6:31   ` Christian Couder
2021-04-02  7:05     ` Chinmoy Chakraborty
2021-04-03 14:14     ` Chinmoy Chakraborty
2021-04-05 14:44       ` Christian Couder
2021-04-06 19:53         ` Chinmoy Chakraborty [this message]
2021-04-08  9:11         ` Chinmoy Chakraborty
2021-04-10 12:03           ` Christian Couder
2021-04-02  7:04   ` Chinmoy Chakraborty

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=f58da7ee-1f35-0e59-a5ac-ae1a268719ed@gmail.com \
    --to=chinmoy12c@gmail.com \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).