Re: [PATCH] git-submodule: respect -q for add/update
On Tue, Sep 4, 2012 at 6:28 PM, Jens Lehmann jens.lehm...@web.de wrote: Am 04.09.2012 09:31, schrieb Orgad Shaneh: Signed-off-by: Orgad Shaneh org...@gmail.com Before the Signed-off-by is the place where you should have explained why this would be a worthwhile change ;-) To me this looks like you make the default noisier and require an explicit -q to make it quiet again. There is a reason you don't normally get bothered with the output of the checkout command run under the hood of git submodule add/update, so I don't think this change makes things better. But you might want to think about adding a -v/--verbose flag to make the submodule add/update checkouts more verbose, in case you care about the output of the checkout command. That would be a sane thing to do, so what about changing your patch into this direction? I don't agree the default should be quiet. That's what the (submodule) -q flag is there for. When I run 'git submodule update' I don't expect to be in the dark until the submodule/s finishes checkout, this sometimes can take a significant amount of time and feedback is expected. - Orgad -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-submodule: respect -q for add/update
Am 05.09.2012 13:42, schrieb Orgad and Raizel Shaneh: On Tue, Sep 4, 2012 at 6:28 PM, Jens Lehmann jens.lehm...@web.de wrote: Am 04.09.2012 09:31, schrieb Orgad Shaneh: Signed-off-by: Orgad Shaneh org...@gmail.com Before the Signed-off-by is the place where you should have explained why this would be a worthwhile change ;-) To me this looks like you make the default noisier and require an explicit -q to make it quiet again. There is a reason you don't normally get bothered with the output of the checkout command run under the hood of git submodule add/update, so I don't think this change makes things better. But you might want to think about adding a -v/--verbose flag to make the submodule add/update checkouts more verbose, in case you care about the output of the checkout command. That would be a sane thing to do, so what about changing your patch into this direction? I don't agree the default should be quiet. That's what the (submodule) -q flag is there for. Nope, the -q flag is to silence *all* output except errors. And it makes perfect sense for high level commands to hide the output of the commands run under the hood like we do here. When I run 'git submodule update' I don't expect to be in the dark until the submodule/s finishes checkout, this sometimes can take a significant amount of time and feedback is expected. As I said, add a verbose flag so you can see in detail what is going on. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] git-submodule: respect -q for add/update
Signed-off-by: Orgad Shaneh org...@gmail.com
---
git-submodule.sh | 17 ++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index aac575e..dd57abb 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -266,6 +266,11 @@ cmd_add()
repo=$1
sm_path=$2
+ quiet=
+ if test -n $GIT_QUIET
+ then
+ quiet=-q
+ fi
if test -z $sm_path; then
sm_path=$(echo $repo |
@@ -332,8 +337,8 @@ Use -f if you really want to add it. 2
cd $sm_path
# ash fails to wordsplit ${branch:+-b $branch...}
case $branch in
- '') git checkout -f -q ;;
- ?*) git checkout -f -q -B $branch origin/$branch ;;
+ '') git checkout -f $quiet ;;
+ ?*) git checkout -f $quiet -B $branch
origin/$branch ;;
esac
) || die $(eval_gettext Unable to checkout submodule
'\$sm_path')
fi
@@ -527,6 +532,12 @@ cmd_update()
shift
done
+ quiet=
+ if test -n $GIT_QUIET
+ then
+ quiet=-q
+ fi
+
if test -n $init
then
cmd_init -- $@ || return
@@ -619,7 +630,7 @@ Maybe you want to use 'update --init'?)
must_die_on_failure=yes
;;
*)
- command=git checkout $subforce -q
+ command=git checkout $subforce $quiet
die_msg=$(eval_gettext Unable to checkout
'\$sha1' in submodule path '\$sm_path')
say_msg=$(eval_gettext Submodule path
'\$sm_path': checked out '\$sha1')
;;
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-submodule: respect -q for add/update
Am 04.09.2012 09:31, schrieb Orgad Shaneh:
Signed-off-by: Orgad Shaneh org...@gmail.com
Before the Signed-off-by is the place where you should have
explained why this would be a worthwhile change ;-)
To me this looks like you make the default noisier and require an
explicit -q to make it quiet again. There is a reason you don't
normally get bothered with the output of the checkout command run
under the hood of git submodule add/update, so I don't think this
change makes things better.
But you might want to think about adding a -v/--verbose flag to
make the submodule add/update checkouts more verbose, in case you
care about the output of the checkout command. That would be a
sane thing to do, so what about changing your patch into this
direction?
---
git-submodule.sh | 17 ++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index aac575e..dd57abb 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -266,6 +266,11 @@ cmd_add()
repo=$1
sm_path=$2
+ quiet=
+ if test -n $GIT_QUIET
+ then
+ quiet=-q
+ fi
if test -z $sm_path; then
sm_path=$(echo $repo |
@@ -332,8 +337,8 @@ Use -f if you really want to add it. 2
cd $sm_path
# ash fails to wordsplit ${branch:+-b $branch...}
case $branch in
- '') git checkout -f -q ;;
- ?*) git checkout -f -q -B $branch origin/$branch ;;
+ '') git checkout -f $quiet ;;
+ ?*) git checkout -f $quiet -B $branch
origin/$branch ;;
esac
) || die $(eval_gettext Unable to checkout submodule
'\$sm_path')
fi
@@ -527,6 +532,12 @@ cmd_update()
shift
done
+ quiet=
+ if test -n $GIT_QUIET
+ then
+ quiet=-q
+ fi
+
if test -n $init
then
cmd_init -- $@ || return
@@ -619,7 +630,7 @@ Maybe you want to use 'update --init'?)
must_die_on_failure=yes
;;
*)
- command=git checkout $subforce -q
+ command=git checkout $subforce $quiet
die_msg=$(eval_gettext Unable to checkout
'\$sha1' in submodule path '\$sm_path')
say_msg=$(eval_gettext Submodule path
'\$sm_path': checked out '\$sha1')
;;
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
