Git Mailing List Archive on lore.kernel.org
 help / color / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Đoàn Trần Công Danh" <congdanhqx@gmail.com>
Cc: Shourya Shukla <shouryashukla.oo@gmail.com>,
	Johannes.Schindelin@gmx.de, chriscool@tuxfamily.org,
	christian.couder@gmail.com, git@vger.kernel.org,
	kaartic.sivaraam@gmail.com, liu.denton@gmail.com,
	sunshine@sunshineco.com
Subject: Re: [GSoC][PATCH v5] submodule: port subcommand 'set-branch' from shell to C
Date: Wed, 03 Jun 2020 13:02:12 -0700
Message-ID: <xmqqtuzrrk8r.fsf@gitster.c.googlers.com> (raw)
In-Reply-To: <20200603001225.GB2222@danh.dev> (=?utf-8?B?IsSQb8OgbiBUcg==?= =?utf-8?B?4bqnbiBDw7RuZw==?= Danh"'s message of "Wed, 3 Jun 2020 07:12:25 +0700")

Đoàn Trần Công Danh  <congdanhqx@gmail.com> writes:

> On 2020-06-02 10:58:45-0700, Junio C Hamano <gitster@pobox.com> wrote:
>> Shourya Shukla <shouryashukla.oo@gmail.com> writes:
>> 
>> > +	 * though there is nothing to make less verbose in this subcommand.
>> > +	 */
>> > +	struct option options[] = {
>> > +		OPT_NOOP_NOARG('q', "quiet"),
>> > +		OPT_BOOL('d', "default", &opt_default,
>> > +			N_("set the default tracking branch to master")),
>> > +		OPT_STRING('b', "branch", &opt_branch, N_("branch"),
>> > +			N_("set the default tracking branch")),
>> > ...
>> > +		OPT_END()
>> > +	};
>> > +	const char *const usage[] = {
>> > +		N_("git submodule--helper set-branch [-q|--quiet] (-d|--default) <path>"),
>> > +		N_("git submodule--helper set-branch [-q|--quiet] (-b|--branch) <branch> <path>"),
>> 
>> 
>> I notice that we gained back -d and -b shorthands that was
>> advertised but not implemented the previous rounds.  It is a bit
>> curious that we are adding these short-hands that nobody uses,
>> though.  
>
> I think a day will come, when all git-submodule functionalities will
> run by calling git-submodule--helper.

I'd expect that when that day with no scripted parts of "git
submodule" remains comes, the main entry point functions in
builtin/submodule--helper.c (like module_list(), update_clone(),
module_set_branch(), etc.) will become helper functions that live in
submodule-lib.c and would be called from builtin/submodule.c.  And
the conversion would rip out calls to parse_options() in each of
these functions that would migrate to submodule-lib.c

    Side note: instead of adding submodule-lib.c, you could add them
    directly to submodule.c if they are small enough.  I am however
    modeling after how the "diff" family was converted to C; the
    diff-lib.c layer is "library-ish helpers that get pre-parsed
    command line arguments and performs a single unit of work" that
    utilizes service routines at the lower layer that are in diff.c
    and submodule-lib.c and submodule.c will be in a similar kind of
    relationship.

> In that day, we will use current git-submodule--helper as the new
> git-submodule.

No, I do not think so.  Most of the option parsers would be redone
in builtin/submodule.c; only some that can be used as-is may migrate
as a whole to builtin/submodule.c and its parse_options() stuff
reused, but most of what is in submodule--helper would have to lose
their parse_options() calls, as nobody would be using module_list()
when there is no scripted "git submodule" exists, for example.

> Or is that a complain for missing some tests?

No, it was "do the minimum necessary for an implementation detail,
as we'll discard that part later anyway".

  reply index

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-21 16:38 [PATCH v3] " Shourya Shukla
2020-05-21 18:44 ` Junio C Hamano
2020-05-21 19:03   ` Denton Liu
2020-05-21 19:50     ` Junio C Hamano
2020-05-22 19:39       ` Shourya Shukla
2020-05-24 16:07         ` Junio C Hamano
2020-05-21 23:04 ` Đoàn Trần Công Danh
2020-05-22 22:21 ` Johannes Schindelin
2020-05-24 23:15   ` Junio C Hamano
2020-05-24 23:18     ` Junio C Hamano
2020-05-23 16:39 ` [PATCH v4] " Shourya Shukla
2020-05-23 18:49   ` Kaartic Sivaraam
2020-05-23 23:18     ` Đoàn Trần Công Danh
2020-05-27 17:13     ` Shourya Shukla
2020-05-28 12:21       ` Đoàn Trần Công Danh
2020-05-28 14:01         ` Đoàn Trần Công Danh
2020-05-28 15:55           ` Đoàn Trần Công Danh
2020-06-02 16:35   ` [GSoC][PATCH v5] " Shourya Shukla
2020-06-02 17:58     ` Junio C Hamano
2020-06-03  0:12       ` Đoàn Trần Công Danh
2020-06-03 20:02         ` Junio C Hamano [this message]
2020-06-04  7:17           ` Shourya Shukla
2020-06-04  7:49             ` Christian Couder
2020-06-04 15:03             ` Junio C Hamano
2020-06-02 19:01     ` Kaartic Sivaraam
2020-06-02 19:10       ` Kaartic Sivaraam
2020-06-02 19:45       ` Christian Couder
2020-06-04  7:09         ` Shourya Shukla
2020-06-04 19:26         ` Kaartic Sivaraam

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

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

  git send-email \
    --in-reply-to=xmqqtuzrrk8r.fsf@gitster.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=chriscool@tuxfamily.org \
    --cc=christian.couder@gmail.com \
    --cc=congdanhqx@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=kaartic.sivaraam@gmail.com \
    --cc=liu.denton@gmail.com \
    --cc=shouryashukla.oo@gmail.com \
    --cc=sunshine@sunshineco.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

Git Mailing List Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/git/0 git/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 git git/ https://lore.kernel.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.git


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git