From: Christian Couder <christian.couder@gmail.com>
To: Shourya Shukla <shouryashukla.oo@gmail.com>
Cc: git <git@vger.kernel.org>, Jeff King <peff@peff.net>,
Heba Waly <heba.waly@gmail.com>, Elijah Newren <newren@gmail.com>,
Johannes Schindelin <Johannes.Schindelin@gmx.de>
Subject: Re: [GSoC][RFC][Proposal v4] Convert submodule to builtin
Date: Tue, 24 Mar 2020 10:08:37 +0100 [thread overview]
Message-ID: <CAP8UFD3BMEqt1+gGgGPHdMQ+BzY6Q84rGcp2UNK=WLusYQuy4g@mail.gmail.com> (raw)
In-Reply-To: <20200323171554.5254-1-shouryashukla.oo@gmail.com>
Hi,
On Mon, Mar 23, 2020 at 6:16 PM Shourya Shukla
<shouryashukla.oo@gmail.com> wrote:
> Prathamesh implemented and improved the subcommands status[https://lore.kernel.org/git/20170713200538.25806-4-pc44800@gmail.com/], sync[https://lore.kernel.org/git/20180114211529.6391-2-pc44800@gmail.com/], deinit[https://lore.kernel.org/git/20180114211529.6391-3-pc44800@gmail.com/] and some more. The relevancy of this to my project is that some helper functions(located in submodule.c) such as print_submodule_summary(),prepare_submodule_summary(), etc. have been implemented beforehand. In the case of subcommand summary, use these functions, integrate them with the basic scaffolding(mentioned in the table below) and implement the module_summary() frontend function.
The last sentence above is not very clear to me. Do you mean "In the
case of subcommand summary, I should use these functions, integrate
them ..."? Or maybe "In the case of subcommand summary, the things
left to do are to use these functions, integrate them ..."?
> He also ported various helper functions such as set_name_rev()[https://lore.kernel.org/git/20170619215025.10086-3-pc44800@gmail.com/]. He kept offering improvements to his conversions till around January of 2018.
Nice!
> Stefan Beller finished the implementation of the subcommand init[https://lore.kernel.org/git/1453418323-29587-1-git-send-email-sbeller@google.com/] as well as laid its foundation[https://lore.kernel.org/git/1453255396-31942-3-git-send-email-sbeller@google.com/]. He implemented foreach[https://lore.kernel.org/git/20180509002952.172347-5-sbeller@google.com/] and improved deinit[https://lore.kernel.org/git/20180327232824.112539-1-sbeller@google.com/] & update[https://lore.kernel.org/git/1444960333-16003-6-git-send-email-sbeller@google.com/] as well. He also ported various helper functions such as resolve_relative_url()[https://lore.kernel.org/git/1460767813-25916-2-git-send-email-sbeller@google.com/].
Nice!
> #### Current Status of the subcommand and future vision
>
> The current status of the conversion as well as the direction I will take for the conversion of the subcommands are as follows:
>
> add: pending conversion, full code needs to be written for the same. Need to implement callback macros and structures, i.e. struct add_cb,
> ADD_CB_INIT, as well as frontend function module_add(). Other helper functions may be needed in the process as well. Compare with shell
> script and try to “translate” it into C. I guesstimate around 400-500 lines of code for this(including helper functions).
Happy to see a guesstimation!
> set-branch: pending conversion, full code needs to be written for the same. Need to implement macros and structures, i.e. struct setbranch,
> SETBRANCH_CB_INIT, as well as frontend function module_setbranch(). Other helper functions(such as remote_submodule_branch() &
> get_default_remote() which are already implemented may prove helpful later) may be needed in the process as well. Compare with shell
> script and try to “translate” it into C. This subcommand may take about 200 lines of C code to implement(including helper functions).
As it looks like it requires less lines of code, do you think it might
be better to do this one before the 'add' sub-command?
> set-url: pending conversion, full code needs to be written for the same. Need to implement macros and structures, i.e. struct seturl,
> SETURL_CB_INIT, as well as frontend function module_seturl(). Other helper functions(such as remote_url() & resolve_resolve_url() which
> are already implemented may prove helpful later) may be needed in the process as well. Compare with shell script and try to “translate” it
> into C. It will have a similar implementation to set-branch because they are “setter” functions. This subcommand may take about 200 lines
> of C code to implement(including helper functions).
>
> summary: pending conversion, work in progress; callback structures, functions and macros have been created, also, basic scaffolding of the command
> is done, i.e., functions module_summary(), summary_submodule(), summary_submodule_cb(). As this is a prototype, some functions may be scrapped
> or added later. Other functions to complement the subcommand have already been created; learn from Prathamesh's mistakes and implement a better code.
> After discussions with Junio C Hamano[https://lore.kernel.org/git/20200318163234.21628-1-shouryashukla.oo@gmail.com/T/#ma3912b761b6deda937691a19c1a070e5e9b34bd7],
> I intend to add a “--recursive” option as well for summary so as to obtain summaries of nested submodules as well.
> I estimate about 400 lines of code for this subcommand(excluding the “--recursive” option, yet including the helper functions)
Nice!
> I aim to follow the same approach as Stefan and Prathamesh as mentioned above.
I am not sure that you talked much about their approach above.
> Though, there is about a 3 year gap between their work and mine, the model for porting seems to be consistent even if coding style may vary and might even give out improvements over previous implementations.
> #### Contribution process and interaction with the mentors
>
> I will keep committing changes on my GitHub fork[https://github.com/periperidip/git] and finally post a bundled patch on the Mailing List.
By "bundled patch" do you mean a patch series?
> I will make sure to keep interacting with the community as well as the mentors regularly. I aim to write weekly “progress report” blogs, which I will post on my website as well as the List. Apart from that, I will document anything new I learn as well as my journey in the GSoC program on my blogs and maybe as self-answered questions on StackOverflow with the aim that they will help me as well as others in case of reference.
>
>
> #### Project Timeline
>
> I have been studying the code of 'submodule.c', 'submodule--helper.c' and 'git-submodule.sh'
> since the submission of my microproject. After studying the codes, I tried to devise an effective
> conversion strategy for 'submodule'. I noticed that 'submodule.c' contains various helper functions
> for 'submodule--helper.c' whereas the latter houses the main "converted" command as of now.
>
> The subcommands ‘set-branch’ and ‘set-url’ will provide easy conversion due to the vast array of helper functions already available for them.
Nice that you found this!
> Hence, I have coupled them with other commands and tasks such as the conversion of ‘add’ and improving CLI parsing.
Hmm.
> In spite of the easiness of the aforementioned conversions, I have decided to convert ‘summary’ first because of the large variety of resources available for it
It is not very clear which resources you are talking about.
> as well as a direction of conversion available(as done by Prathamesh)
Do you mean that he already sent patches or wrote commits in his repo
about that?
> which will aid me in learning from the mistakes committed before and thus help me offer an even more improved version of the subcommand.
Thanks,
Christian.
prev parent reply other threads:[~2020-03-24 9:08 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-23 17:15 [GSoC][RFC][Proposal v4] Convert submodule to builtin Shourya Shukla
2020-03-24 9:08 ` Christian Couder [this message]
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='CAP8UFD3BMEqt1+gGgGPHdMQ+BzY6Q84rGcp2UNK=WLusYQuy4g@mail.gmail.com' \
--to=christian.couder@gmail.com \
--cc=Johannes.Schindelin@gmx.de \
--cc=git@vger.kernel.org \
--cc=heba.waly@gmail.com \
--cc=newren@gmail.com \
--cc=peff@peff.net \
--cc=shouryashukla.oo@gmail.com \
/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).