From: Jonathan Nieder <jrnieder@gmail.com>
To: Jens Lehmann <Jens.Lehmann@web.de>
Cc: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>,
git <git@vger.kernel.org>, Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH] mv: Fix spurious warning when moving a file in presence of submodules
Date: Thu, 17 Oct 2013 11:24:05 -0700 [thread overview]
Message-ID: <20131017182405.GR9464@google.com> (raw)
In-Reply-To: <525A8965.3040407@web.de>
Jens Lehmann wrote:
> In commit 0656781fa "git mv" learned to update the submodule path in the
> .gitmodules file when moving a submodule in the work tree. But since that
> commit update_path_in_gitmodules() gets called no matter if we moved a
> submodule or a regular file, which is wrong and leads to a bogus warning
> when moving a regular file in a repo containing a .gitmodules file:
>
> warning: Could not find section in .gitmodules where path=<filename>
[...]
> Reported-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
> Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
> ---
[...]
> And this is the fix for it, which I believe is stuff for maint.
Thanks again, and sorry to leave this hanging. I had some vague ideas
for improvement:
* style nits: test is a little long, making it hard to take in at a
glance; tests tend to try to avoid pipelines to avoid losing the
exit code from git commands; usual style is to use "test" instead
of "[" consistently
* would be nice to have another test that makes sure the "move a
file, not submodule" case keeps working
But those can easily happen as changes on top, and as you mentioned,
it's probably worth someone interested (e.g., me :)) making a pass to
clean up the style of the rest of the script anyway.
The patch as-is is obviously good. Thanks again for the quick
diagnosis and fix.
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
prev parent reply other threads:[~2013-10-17 18:24 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-10-11 14:29 Spurious warning when moving a file in presence of submodules Matthieu Moy
2013-10-11 17:53 ` Jens Lehmann
2013-10-13 11:52 ` [PATCH] mv: Fix spurious " Jens Lehmann
2013-10-13 15:05 ` Matthieu Moy
2013-10-13 18:37 ` Jens Lehmann
2013-10-14 5:40 ` Jonathan Nieder
2013-10-14 12:33 ` Matthieu Moy
2013-10-17 18:24 ` Jonathan Nieder [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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20131017182405.GR9464@google.com \
--to=jrnieder@gmail.com \
--cc=Jens.Lehmann@web.de \
--cc=Matthieu.Moy@grenoble-inp.fr \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.