On 05/08 02:36, Junio C Hamano wrote:
> Shourya Shukla <shouryashukla.oo@xxxxxxxxx> 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.
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?
> > NEEDSWORK regarding the outdated syntax and working of the test, which
> > may need to be improved to obtain better and desired results.
>
> Sounds good.
>
> > While at it, change the word 'test' to 'test script' in the test
> > description to avoid ambiguity.
>
> Sounds good. I often search for a pair of phrases to differentiate
> a single tXXXX-name.sh file as a whole and an individual test piece
> in it. "This test script", especially when written near the
> beginning of the file, is a good way to clearly convey that you want
> to refer to the former.
>
> > Mentored-by: Christian Couder <chriscool@xxxxxxxxxxxxx>
> > Mentored-by: Kaartic Sivaraam <kaartic.sivaraam@xxxxxxxxx>
> > Signed-off-by: Shourya Shukla <shouryashukla.oo@xxxxxxxxx>
> > ---
> > t/t7401-submodule-summary.sh | 7 ++++++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/t/t7401-submodule-summary.sh b/t/t7401-submodule-summary.sh
> > index 145914cd5a..2db4cf5cbf 100755
> > --- a/t/t7401-submodule-summary.sh
> > +++ b/t/t7401-submodule-summary.sh
> > @@ -5,8 +5,13 @@
> >
> > test_description='Summary support for submodules
> >
> > -This test tries to verify the sanity of summary subcommand of git submodule.
> > +This test script tries to verify the sanity of summary subcommand of git submodule.
> > '
> > +# WARNING: This test script uses 'git add' instead of 'git submodule add' to add
> > +# submodules to the superproject. Some submodule subcommands such as init and
> > +# deinit might not work as expected in this script.
> > +
> > +# NEEDSWORK: This test script is old fashioned and may need a big cleanup.
> >
> > . ./test-lib.sh
