git@vger.kernel.org mailing list mirror (one of many)
 help / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Prathamesh Chavan <pc44800@gmail.com>
Cc: christian.couder@gmail.com, git@vger.kernel.org, sbeller@google.com
Subject: Re: [GSoC][PATCH 1/6 v2] submodule--helper: introduce for_each_submodule_list
Date: Wed, 28 Jun 2017 12:53:34 -0700
Message-ID: <xmqq60ffx51d.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20170626231108.23640-1-pc44800@gmail.com>

Prathamesh Chavan <pc44800@gmail.com> writes:

> Introduce function for_each_submodule_list for using it
> in the later patches, related to porting submodule
> subcommands from shell to C.
> This new function is also used in ported submodule subcommand
> init.

The patch text looks sensible.  It would be easier for "git log"
readers to understand, if the change is explained like so:

	Introduce function for_each_submodule_list() and
	replace a loop in module_init() with a call to it.

	The new function will also be used in other parts of the
	system in later patches.

That way, readers do not have to judge the merit of this change
based on a vague promise "it will help world better with future
patches", but can instead judge on its immediate benefit that it
refactors a useful bit out of an existing code.

> Mentored-by: Christian Couder <christian.couder@gmail.com>
> Mentored-by: Stefan Beller <sbeller@google.com>
> Signed-off-by: Prathamesh Chavan <pc44800@gmail.com>
> ---
> This series of patches is based on the 'next' branch. 

The reason not to base on 'master' is...?

The thing is that a topic built on 'next' cannot be merged down to
'master' until _all_ other topics in 'next' graduate to 'master',
which may never happen.  If you are depending on one or more topics,
please make sure to name them.  Then we can

 (1) create a branch from the tip of 'master';
 (2) merge these topics you depend on into that branch; and then
 (3) apply these patches.

The topic still needs to wait until these other topis graduate, but
at least you would not be blocked by unrelated topics that way.

You _might_ be building on 'next' because you want to make sure that
your topic works not just with master but also want to make sure
that there won't be any unexpected breakage when used with topics in
'next', even though your topic does not depend on anything in 'next'
in particular.  It is a good development discipline to pay attention
to other topics in flight and I applaud you for it if that is why
you based it on 'next'.  But the right way to do it would be to
build your topic on 'master', and then in addition to testing the
topic by itself, also make a trial merge of your topic into 'next'
and test the result as well.

Thanks.

  parent reply index

Thread overview: 28+ messages in thread (expand / mbox.gz / Atom feed / [top])
2017-06-26 23:09 [GSoC] Update: Week 6 Prathamesh Chavan
2017-06-26 23:11 ` [GSoC][PATCH 1/6 v2] submodule--helper: introduce for_each_submodule_list Prathamesh Chavan
2017-06-26 23:11   ` [GSoC][PATCH 2/6 v2] submodule: port subcommand foreach from shell to C Prathamesh Chavan
2017-06-27  6:06     ` Christian Couder
2017-06-26 23:11   ` [GSoC][PATCH 3/6 v2] submodule: port set_name_rev " Prathamesh Chavan
2017-06-26 23:11   ` [GSoC][PATCH 4/6 v2] submodule: port submodule subcommand status Prathamesh Chavan
2017-06-26 23:11   ` [GSoC][PATCH 5/6 v2] submodule: port submodule subcommand sync from shell to C Prathamesh Chavan
2017-06-27  6:37     ` Christian Couder
2017-06-26 23:11   ` [GSoC][PATCH 6/6 v2] submodule: port submodule subcommand 'deinit' " Prathamesh Chavan
2017-06-27  7:18     ` Christian Couder
2017-06-28 19:53   ` Junio C Hamano [this message]
2017-06-29 11:47     ` [GSoC][PATCH 1/6 v2] submodule--helper: introduce for_each_submodule_list Prathamesh Chavan
2017-06-30 19:47 ` [GSoC][PATCH 1/5 v3] submodule--helper: introduce get_submodule_displaypath() Prathamesh Chavan
2017-06-30 19:47   ` [GSoC][PATCH 2/5 v3] submodule--helper: introduce for_each_submodule_list() Prathamesh Chavan
2017-06-30 19:47   ` [GSoC][PATCH 3/5 v3] submodule: port set_name_rev() from shell to C Prathamesh Chavan
2017-06-30 22:49     ` Junio C Hamano
2017-06-30 19:47   ` [GSoC][PATCH 4/5 v3] submodule: port submodule subcommand 'status' " Prathamesh Chavan
2017-06-30 23:08     ` Junio C Hamano
2017-06-30 19:47   ` [GSoC][PATCH 5/5 v3] submodule: port submodule subcommand 'sync' " Prathamesh Chavan
2017-06-30 20:11     ` Stefan Beller
2017-06-30 23:19     ` Junio C Hamano
2017-06-30 22:37   ` [GSoC][PATCH 1/5 v3] submodule--helper: introduce get_submodule_displaypath() Junio C Hamano
2017-07-13 20:05   ` [GSoC][PATCH 1/5 v4] " Prathamesh Chavan
2017-07-13 20:05     ` [GSoC][PATCH 2/5 v4] submodule--helper: introduce for_each_submodule_list() Prathamesh Chavan
2017-07-13 20:05     ` [GSoC][PATCH 3/5 v4] submodule: port set_name_rev() from shell to C Prathamesh Chavan
2017-07-13 20:05     ` [GSoC][PATCH 4/5 v4] submodule: port submodule subcommand 'status' " Prathamesh Chavan
2017-07-13 22:44       ` Stefan Beller
2017-07-13 20:05     ` [GSoC][PATCH 5/5 v4] submodule: port submodule subcommand 'sync' " Prathamesh Chavan

Reply instructions:

You may reply publically 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 to all the recipients using the --to, --cc,
  and --in-reply-to switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqq60ffx51d.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=pc44800@gmail.com \
    --cc=sbeller@google.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@vger.kernel.org mailing list mirror (one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/
       or Tor2web: https://www.tor2web.org/

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