git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: Christian Couder <christian.couder@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Shourya Shukla <shouryashukla.oo@gmail.com>,
	git <git@vger.kernel.org>,
	Kaartic Sivaraam <kaartic.sivaraam@gmail.com>,
	Johannes Schindelin <johannes.schindelin@gmx.de>,
	Christian Couder <chriscool@tuxfamily.org>,
	Taylor Blau <me@ttaylorr.com>, Denton Liu <liu.denton@gmail.com>
Subject: Re: [PATCH 4/4] t7401: add a WARNING and a NEEDSWORK
Date: Fri, 7 Aug 2020 16:55:11 +0200	[thread overview]
Message-ID: <CAP8UFD21i6UOwSA8hoCbF9kbRaaQzmN4e0t+pPrF1fW+SSJwww@mail.gmail.com> (raw)
In-Reply-To: <xmqqa6z7jx7q.fsf@gitster.c.googlers.com>

On Thu, Aug 6, 2020 at 11:11 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Shourya Shukla <shouryashukla.oo@gmail.com> writes:
>
> > On 05/08 02:36, Junio C Hamano wrote:
> >> Shourya Shukla <shouryashukla.oo@gmail.com> writes:
> >>
> >> > Add a WARNING regarding the usage of 'git add' instead of 'git
> >> > submodule add' to add submodules to the superproject.
> >>
> >> Is that a warning worthy thing?  As far as I know, using "git add"
> >> to register a gitlink is perfectly fine and a supported way to start
> >> a new submodule.  It may have to be followed by other steps like
> >> "git config -f .gitmodules" (e.g. when operations that needs to use
> >> the contents recorded in the .gitmodules file are to be tested), but
> >> writing tests using lower-level ingredients for finer grained tests
> >> is not all that unusual, is it?  I dunno.
> >
> > The thing is that 'git submodule {init,deinit}' fail when there is no
> > .gitmodules. I can initiliase the .gitmodules separately using 'git
> > config -f .gitmodules' but I think it will be better to use 'git
> > submodule add' throughout the script rather than worry about it all the
> > time.
>
> On the other hand, we do want to make sure that the workflow using
> lower-level tools continues to work, so that is a balancing act.

Yeah, that's the reason why we suggested to add the new tests in a new
test script t7421:

https://lore.kernel.org/git/20200806164102.6707-5-shouryashukla.oo@gmail.com/

> > But again, if the warning seems unnecessary, then I can obviously use
> > 'git config' to initilaise the submodules and change this commit. What
> > do you reckon?
>
> If you need "git submodule init" etc. to work in this test, yes, you
> can change the test to either use "git submodule add" instead, or
> "git config -f .gitmodules" in addition.  If you don't, there is no
> need to change anything, no?

In t7421 "git submodule add" is used, so that we can perform the new
tests we want there. The purpose of this patch adding a WARNING and a
NEEDSWORK was to tell explicitly that this test script (t7401) is not
necessarily the best test script for "git submodule summary" tests
because it is very old fashioned as it has a lot of boilerplate stuff
outside test_expect_{success, failure} and it uses "git add" instead
of "git submodule add".

I think we might want to just add the NEEDSWORK in this patch series
and add the WARNING, or perhaps just a NOTE, about "git add" vs "git
submodule add" in the other patch series when we introduce t7421.

  reply	other threads:[~2020-08-07 14:55 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-05 17:49 [GSoC][RESEND][PATCH 0/4] t7401: modernize, cleanup and warn Shourya Shukla
2020-08-05 17:49 ` [PATCH 1/4] t7401: modernize style Shourya Shukla
2020-08-05 19:37   ` Denton Liu
2020-08-05 20:49     ` Taylor Blau
2020-08-06  8:45       ` Shourya Shukla
2020-08-05 17:49 ` [PATCH 2/4] t7401: change test_i18ncmp syntax for clarity Shourya Shukla
2020-08-05 20:58   ` Taylor Blau
2020-08-05 21:23   ` Junio C Hamano
2020-08-05 17:49 ` [PATCH 3/4] t7401: ensure uniformity in the '--for-status' test Shourya Shukla
2020-08-05 21:01   ` Taylor Blau
2020-08-05 22:25     ` Junio C Hamano
2020-08-05 22:26       ` Taylor Blau
2020-08-05 21:30   ` Junio C Hamano
2020-08-06  8:50     ` Shourya Shukla
2020-08-06 17:18       ` Junio C Hamano
2020-08-05 17:49 ` [PATCH 4/4] t7401: add a WARNING and a NEEDSWORK Shourya Shukla
2020-08-05 21:04   ` Taylor Blau
2020-08-05 21:36   ` Junio C Hamano
2020-08-06 11:27     ` Shourya Shukla
2020-08-06 21:11       ` Junio C Hamano
2020-08-07 14:55         ` Christian Couder [this message]
2020-08-12 19:27 ` [GSoC][PATCH v2 0/4] t7401: modernize, cleanup and more Shourya Shukla
2020-08-12 19:27   ` [PATCH v2 1/4] t7401: modernize style Shourya Shukla
2020-08-13  8:06     ` Kaartic Sivaraam
2020-08-13 16:46       ` Junio C Hamano
2020-08-14 14:41         ` Shourya Shukla
2020-08-14 17:06           ` Junio C Hamano
2020-08-12 19:27   ` [PATCH v2 2/4] t7401: change test_i18ncmp syntax for clarity Shourya Shukla
2020-08-12 20:57     ` Junio C Hamano
2020-08-12 21:02       ` Junio C Hamano
2020-08-14 14:57         ` Shourya Shukla
2020-08-12 19:27   ` [PATCH v2 3/4] t7401: change indentation for enhanced readability Shourya Shukla
2020-08-12 19:27   ` [PATCH v2 4/4] t7401: add a NEEDSWORK Shourya Shukla
  -- strict thread matches above, loose matches on Subject: below --
2020-07-26 14:25 [GSoC][PATCH 0/4] t7401: modernize, cleanup and warn Shourya Shukla
2020-07-26 14:25 ` [PATCH 4/4] t7401: add a WARNING and a NEEDSWORK Shourya Shukla

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=CAP8UFD21i6UOwSA8hoCbF9kbRaaQzmN4e0t+pPrF1fW+SSJwww@mail.gmail.com \
    --to=christian.couder@gmail.com \
    --cc=chriscool@tuxfamily.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=johannes.schindelin@gmx.de \
    --cc=kaartic.sivaraam@gmail.com \
    --cc=liu.denton@gmail.com \
    --cc=me@ttaylorr.com \
    --cc=shouryashukla.oo@gmail.com \
    --subject='Re: [PATCH 4/4] t7401: add a WARNING and a NEEDSWORK' \
    /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 list mirror (unofficial, one of many)

This inbox may be cloned and mirrored by anyone:

	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

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

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://7fh6tueqddpjyxjmgtdiueylzoqt6pt7hec3pukyptlmohoowvhde4yd.onion/inbox.comp.version-control.git
	nntp://ie5yzdi7fg72h7s4sdcztq5evakq23rdt33mfyfcddc5u3ndnw24ogqd.onion/inbox.comp.version-control.git
	nntp://4uok3hntl7oi7b4uf4rtfwefqeexfzil2w6kgk2jn5z2f764irre7byd.onion/inbox.comp.version-control.git
	nntp://news.gmane.io/gmane.comp.version-control.git
 note: .onion URLs require Tor: https://www.torproject.org/

code repositories for project(s) associated with this inbox:

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

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