Re: [PATCH 06/10] git submodule update: Redirect any output to stderr

2015-09-17 Thread Eric Sunshine
On Wed, Sep 16, 2015 at 9:39 PM, Stefan Beller  wrote:
> git submodule update: Redirect any output to stderr

This commit message seems to be lacking an explanation of why this is
being done.

> There are no tests, which fail by this.

Not sure what this means. I suppose you're trying to say that this
patch doesn't break any existing tests, but isn't that an implied goal
of all patches posted to this list?

> Signed-off-by: Stefan Beller 
> ---
>  git-submodule.sh | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 8b0eb9a..7ef3247 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -663,7 +663,7 @@ cmd_update()
> die_if_unmatched "$mode"
> if test "$stage" = U
> then
> -   echo >&2 "Skipping unmerged submodule $prefix$sm_path"
> +   say >&2 "Skipping unmerged submodule $prefix$sm_path"
> continue
> fi
> name=$(git submodule--helper name "$sm_path") || exit
> @@ -684,7 +684,7 @@ cmd_update()
>
> if test "$update_module" = "none"
> then
> -   echo "Skipping submodule '$displaypath'"
> +   say >&2 "Skipping submodule '$displaypath'"

These changes seem to be doing more than what the commit message
claims. The changed code isn't just redirecting to stderr, but is also
now respecting $GIT_QUIET.

> continue
> fi
>
> --
> 2.6.0.rc0.131.gf624c3d
--
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 06/10] git submodule update: Redirect any output to stderr

2015-09-17 Thread Stefan Beller
On Thu, Sep 17, 2015 at 1:31 PM, Eric Sunshine  wrote:
> On Wed, Sep 16, 2015 at 9:39 PM, Stefan Beller  wrote:
>> git submodule update: Redirect any output to stderr
>
> This commit message seems to be lacking an explanation of why this is
> being done.
>
>> There are no tests, which fail by this.
>
> Not sure what this means. I suppose you're trying to say that this
> patch doesn't break any existing tests, but isn't that an implied goal
> of all patches posted to this list?

Yes they should (but they don't yet).

What I was trying to say:

In a reroll I want to add tests to this as I was surprised
of not breaking a test by this commit (so the behavior
of this is untested which is bad)

>
>> Signed-off-by: Stefan Beller 
>> ---
>>  git-submodule.sh | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/git-submodule.sh b/git-submodule.sh
>> index 8b0eb9a..7ef3247 100755
>> --- a/git-submodule.sh
>> +++ b/git-submodule.sh
>> @@ -663,7 +663,7 @@ cmd_update()
>> die_if_unmatched "$mode"
>> if test "$stage" = U
>> then
>> -   echo >&2 "Skipping unmerged submodule 
>> $prefix$sm_path"
>> +   say >&2 "Skipping unmerged submodule $prefix$sm_path"
>> continue
>> fi
>> name=$(git submodule--helper name "$sm_path") || exit
>> @@ -684,7 +684,7 @@ cmd_update()
>>
>> if test "$update_module" = "none"
>> then
>> -   echo "Skipping submodule '$displaypath'"
>> +   say >&2 "Skipping submodule '$displaypath'"
>
> These changes seem to be doing more than what the commit message
> claims. The changed code isn't just redirecting to stderr, but is also
> now respecting $GIT_QUIET.

Right, I need to redo the commit message anyways, so I'll mention that.

>
>> continue
>> fi
>>
>> --
>> 2.6.0.rc0.131.gf624c3d
--
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 06/10] git submodule update: Redirect any output to stderr

2015-09-16 Thread Stefan Beller
There are no tests, which fail by this.

Signed-off-by: Stefan Beller 
---
 git-submodule.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index 8b0eb9a..7ef3247 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -663,7 +663,7 @@ cmd_update()
die_if_unmatched "$mode"
if test "$stage" = U
then
-   echo >&2 "Skipping unmerged submodule $prefix$sm_path"
+   say >&2 "Skipping unmerged submodule $prefix$sm_path"
continue
fi
name=$(git submodule--helper name "$sm_path") || exit
@@ -684,7 +684,7 @@ cmd_update()
 
if test "$update_module" = "none"
then
-   echo "Skipping submodule '$displaypath'"
+   say >&2 "Skipping submodule '$displaypath'"
continue
fi
 
-- 
2.6.0.rc0.131.gf624c3d

--
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