git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [[PATCH][Outreachy]] t7011-skip-worktree-reading.sh: ensure no whitespace after redirect operators
@ 2024-10-18 19:17 Seyi Kuforiji
  2024-10-18 20:04 ` Kristoffer Haugsbakk
  0 siblings, 1 reply; 4+ messages in thread
From: Seyi Kuforiji @ 2024-10-18 19:17 UTC (permalink / raw)
  To: git; +Cc: Patrick Steinhardt, Phillip Wood, Seyi Kuforiji

As discussed in the thread on lore.kernel.org (link below), it is important
to ensure there is no whitespace after redirect operators. This change updates
the script to conform to this standard, changing instances like:

    foo > actual &&

to:

    foo >actual &&

Reference: https://lore.kernel.org/git/CAPig+cQpUu2UO-+jWn1nTaDykWnxwuEitzVB7PnW2SS_b7V8Hg@mail.gmail.com/

Signed-off-by: Seyi Kuforiji <kuforiji98@gmail.com>
---
 t/t7011-skip-worktree-reading.sh | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/t/t7011-skip-worktree-reading.sh b/t/t7011-skip-worktree-reading.sh
index 4adac5acd5..c86abd99bf 100755
--- a/t/t7011-skip-worktree-reading.sh
+++ b/t/t7011-skip-worktree-reading.sh
@@ -32,24 +32,24 @@ setup_absent() {
 }
 
 test_absent() {
-	echo "100644 $EMPTY_BLOB 0	1" > expected &&
-	git ls-files --stage 1 > result &&
+	echo "100644 $EMPTY_BLOB 0	1" >expected &&
+	git ls-files --stage 1 >result &&
 	test_cmp expected result &&
 	test ! -f 1
 }
 
 setup_dirty() {
 	git update-index --force-remove 1 &&
-	echo dirty > 1 &&
+	echo dirty >1 &&
 	git update-index --add --cacheinfo 100644 $EMPTY_BLOB 1 &&
 	git update-index --skip-worktree 1
 }
 
 test_dirty() {
-	echo "100644 $EMPTY_BLOB 0	1" > expected &&
-	git ls-files --stage 1 > result &&
+	echo "100644 $EMPTY_BLOB 0	1" >expected &&
+	git ls-files --stage 1 >result &&
 	test_cmp expected result &&
-	echo dirty > expected
+	echo dirty >expected
 	test_cmp expected 1
 }
 
@@ -59,7 +59,7 @@ test_expect_success 'setup' '
 	touch ./1 ./2 sub/1 sub/2 &&
 	git add 1 2 sub/1 sub/2 &&
 	git update-index --skip-worktree 1 sub/1 &&
-	git ls-files -t > result &&
+	git ls-files -t >result &&
 	test_cmp expect.skip result
 '
 
@@ -86,7 +86,7 @@ test_expect_success 'update-index --remove' '
 	setup_dirty &&
 	git update-index --remove 1 &&
 	test -z "$(git ls-files 1)" &&
-	echo dirty > expected &&
+	echo dirty >expected &&
 	test_cmp expected 1
 '
 
@@ -110,16 +110,16 @@ test_expect_success 'ls-files --modified' '
 	test -z "$(git ls-files -m)"
 '
 
-echo ":000000 100644 $ZERO_OID $EMPTY_BLOB A	1" > expected
+echo ":000000 100644 $ZERO_OID $EMPTY_BLOB A	1" >expected
 test_expect_success 'diff-index does not examine skip-worktree absent entries' '
 	setup_absent &&
-	git diff-index HEAD -- 1 > result &&
+	git diff-index HEAD -- 1 >result &&
 	test_cmp expected result
 '
 
 test_expect_success 'diff-index does not examine skip-worktree dirty entries' '
 	setup_dirty &&
-	git diff-index HEAD -- 1 > result &&
+	git diff-index HEAD -- 1 >result &&
 	test_cmp expected result
 '
 
-- 
2.47.0.86.g15030f9556


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [[PATCH][Outreachy]] t7011-skip-worktree-reading.sh: ensure no whitespace after redirect operators
  2024-10-18 19:17 [[PATCH][Outreachy]] t7011-skip-worktree-reading.sh: ensure no whitespace after redirect operators Seyi Kuforiji
@ 2024-10-18 20:04 ` Kristoffer Haugsbakk
  2024-10-18 21:03   ` Taylor Blau
  0 siblings, 1 reply; 4+ messages in thread
From: Kristoffer Haugsbakk @ 2024-10-18 20:04 UTC (permalink / raw)
  To: Seyi Kuforiji, git; +Cc: Patrick Steinhardt, Phillip Wood

Hiya

> [[PATCH][Outreachy]]

Apparently you can’t nest brackets like this according to git-am(1).  I
got this:

    ] t7011-skip-worktree-reading.sh: ensure no whitespace after redirect operators

Doesn’t really matter though.  I suspect `[PATCH Outreachy]` would work.

On Fri, Oct 18, 2024, at 21:17, Seyi Kuforiji wrote:
> As discussed in the thread on lore.kernel.org (link below), it is important

This is documented in Documentation/CodingGuidelines at “Redirection
operators”.  That’s a more straightforward reference.

> to ensure there is no whitespace after redirect operators. This change updates
> the script to conform to this standard, changing instances like:
>
>     foo > actual &&
>
> to:
>
>     foo >actual &&

We can see that from the patch.  Saying what it does is redundant in
this case in my opinion. :)

I think it suffices to say that you are fixing the code style.  If so
this would have been enough:

> As discussed in the thread on lore.kernel.org (link below), it is important
> to ensure there is no whitespace after redirect operators.

...

You seem to be wrapping the lines at 80 columns.  72 columns is more
common here.  The idea is (I think) that you add some slack for things
like commit message indentation in git-log(1), multiple levels of email
quoting and so on.

It’s kind of indirectly mentioned in Documentation/MyFirstContribution.
I also found this:

https://lore.kernel.org/git/ZrCdDHqKfwWbr_Zn@tanuki/

>
> Reference:
> https://lore.kernel.org/git/CAPig+cQpUu2UO-+jWn1nTaDykWnxwuEitzVB7PnW2SS_b7V8Hg@mail.gmail.com/
>
> Signed-off-by: Seyi Kuforiji <kuforiji98@gmail.com>
> ---
>  t/t7011-skip-worktree-reading.sh | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/t/t7011-skip-worktree-reading.sh b/t/t7011-skip-worktree-reading.sh
> index 4adac5acd5..c86abd99bf 100755
> --- a/t/t7011-skip-worktree-reading.sh
> +++ b/t/t7011-skip-worktree-reading.sh
> @@ -32,24 +32,24 @@ setup_absent() {
>  }
>
>  test_absent() {
> -	echo "100644 $EMPTY_BLOB 0	1" > expected &&
> -	git ls-files --stage 1 > result &&
> +	echo "100644 $EMPTY_BLOB 0	1" >expected &&
> +	git ls-files --stage 1 >result &&
>  	test_cmp expected result &&
>  	test ! -f 1
>  }
>
>  setup_dirty() {
>  	git update-index --force-remove 1 &&
> -	echo dirty > 1 &&
> +	echo dirty >1 &&
>  	git update-index --add --cacheinfo 100644 $EMPTY_BLOB 1 &&
>  	git update-index --skip-worktree 1
>  }
>
>  test_dirty() {
> -	echo "100644 $EMPTY_BLOB 0	1" > expected &&
> -	git ls-files --stage 1 > result &&
> +	echo "100644 $EMPTY_BLOB 0	1" >expected &&
> +	git ls-files --stage 1 >result &&
>  	test_cmp expected result &&
> -	echo dirty > expected
> +	echo dirty >expected
>  	test_cmp expected 1
>  }
>
> @@ -59,7 +59,7 @@ test_expect_success 'setup' '
>  	touch ./1 ./2 sub/1 sub/2 &&
>  	git add 1 2 sub/1 sub/2 &&
>  	git update-index --skip-worktree 1 sub/1 &&
> -	git ls-files -t > result &&
> +	git ls-files -t >result &&
>  	test_cmp expect.skip result
>  '
>
> @@ -86,7 +86,7 @@ test_expect_success 'update-index --remove' '
>  	setup_dirty &&
>  	git update-index --remove 1 &&
>  	test -z "$(git ls-files 1)" &&
> -	echo dirty > expected &&
> +	echo dirty >expected &&
>  	test_cmp expected 1
>  '
>
> @@ -110,16 +110,16 @@ test_expect_success 'ls-files --modified' '
>  	test -z "$(git ls-files -m)"
>  '
>
> -echo ":000000 100644 $ZERO_OID $EMPTY_BLOB A	1" > expected
> +echo ":000000 100644 $ZERO_OID $EMPTY_BLOB A	1" >expected
>  test_expect_success 'diff-index does not examine skip-worktree absent
> entries' '
>  	setup_absent &&
> -	git diff-index HEAD -- 1 > result &&
> +	git diff-index HEAD -- 1 >result &&
>  	test_cmp expected result
>  '
>
>  test_expect_success 'diff-index does not examine skip-worktree dirty entries' '
>  	setup_dirty &&
> -	git diff-index HEAD -- 1 > result &&
> +	git diff-index HEAD -- 1 >result &&
>  	test_cmp expected result
>  '
>
> --
> 2.47.0.86.g15030f9556

The diff here is clean.  It only does what you describe in the commit
message.  Good.

I checked the file and I can’t find any missed instances.  Nice!

-- 
Kristoffer Haugsbakk

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [[PATCH][Outreachy]] t7011-skip-worktree-reading.sh: ensure no whitespace after redirect operators
  2024-10-18 20:04 ` Kristoffer Haugsbakk
@ 2024-10-18 21:03   ` Taylor Blau
  2024-10-19  8:42     ` Seyi Chamber
  0 siblings, 1 reply; 4+ messages in thread
From: Taylor Blau @ 2024-10-18 21:03 UTC (permalink / raw)
  To: Kristoffer Haugsbakk; +Cc: Seyi Kuforiji, git, Patrick Steinhardt, Phillip Wood

On Fri, Oct 18, 2024 at 10:04:43PM +0200, Kristoffer Haugsbakk wrote:
> Hiya
>
> > [[PATCH][Outreachy]]
>
> Apparently you can’t nest brackets like this according to git-am(1).  I
> got this:
>
>     ] t7011-skip-worktree-reading.sh: ensure no whitespace after redirect operators
>
> Doesn’t really matter though.  I suspect `[PATCH Outreachy]` would work.

Indeed. With format-patch, you can do '--rfc=-Outreachy' to achieve the
desired effect.

> The diff here is clean.  It only does what you describe in the commit
> message.  Good.
>
> I checked the file and I can’t find any missed instances.  Nice!

Thanks for the helpful review and feedback for Seyi's patch.

Seyi: I'll queue this into my tree, but mark it as expecting a new
version to address Kristoffer's comments above. Thanks.

Thanks,
Taylor

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [[PATCH][Outreachy]] t7011-skip-worktree-reading.sh: ensure no whitespace after redirect operators
  2024-10-18 21:03   ` Taylor Blau
@ 2024-10-19  8:42     ` Seyi Chamber
  0 siblings, 0 replies; 4+ messages in thread
From: Seyi Chamber @ 2024-10-19  8:42 UTC (permalink / raw)
  To: Taylor Blau; +Cc: Kristoffer Haugsbakk, git, Patrick Steinhardt, Phillip Wood

On Fri, 18 Oct 2024 at 22:03, Taylor Blau <me@ttaylorr.com> wrote:
>
> On Fri, Oct 18, 2024 at 10:04:43PM +0200, Kristoffer Haugsbakk wrote:
>> We can see that from the patch.  Saying what it does is redundant in
>> this case in my opinion. :)

>> I think it suffices to say that you are fixing the code style.  If so
>> this would have been enough:
>> You seem to be wrapping the lines at 80 columns.  72 columns is more
>> common here.  The idea is (I think) that you add some slack for things
>> like commit message indentation in git-log(1), multiple levels of email
>> quoting and so on.

>> It’s kind of indirectly mentioned in Documentation/MyFirstContribution.
>> I also found this:

>> https://lore.kernel.org/git/ZrCdDHqKfwWbr_Zn@tanuki/

Duly noted, haha!

> > Hiya
> >
> > > [[PATCH][Outreachy]]
> >
> > Apparently you can’t nest brackets like this according to git-am(1).  I
> > got this:
> >
> >     ] t7011-skip-worktree-reading.sh: ensure no whitespace after redirect operators
> >
> > Doesn’t really matter though.  I suspect `[PATCH Outreachy]` would work.
>
> Indeed. With format-patch, you can do '--rfc=-Outreachy' to achieve the
> desired effect.

Thank you for the feedback Kristoffer and Taylor!

I'll adjust the subject line to conform to git-am using Taylor's
suggestion for my next patch :)

>
> > The diff here is clean.  It only does what you describe in the commit
> > message.  Good.
> >
> > I checked the file and I can’t find any missed instances.  Nice!
>
> Thanks for the helpful review and feedback for Seyi's patch.
>
> Seyi: I'll queue this into my tree, but mark it as expecting a new
> version to address Kristoffer's comments above. Thanks.
>
> Thanks,
> Taylor

Duly noted, Thanks.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2024-10-19  8:43 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-18 19:17 [[PATCH][Outreachy]] t7011-skip-worktree-reading.sh: ensure no whitespace after redirect operators Seyi Kuforiji
2024-10-18 20:04 ` Kristoffer Haugsbakk
2024-10-18 21:03   ` Taylor Blau
2024-10-19  8:42     ` Seyi Chamber

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).