threads / patch / 61173

patch, 2 partst9803-git-p4-shell-metachars.sh: update to use test_path_* functions

Subject: [PATCH 0/2] t9803-git-p4-shell-metachars.sh: update to use test_path_* functions

## tl;dr

9 messages between Mar 21, 2024 and Mar 21, 2024. Diffs are folded; open one to read it.

replies: 8people: 4as markdown or json

Sanchit Jindal via GitGitGadget· Mar 21, 2024, 19:39 UTC · lore
Correct Typo in Branch name
Sanchit Jindal (1):
  t9803: update commit messages and description
sanchit1053 (1):
  t9803-git-p4-shell-metachars.sh: update to use test_path_* functions
 t/t9803-git-p4-shell-metachars.sh | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
base-commit: 3bd955d26919e149552f34aacf8a4e6368c26cec
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1691%2Fsanchit1053%2Fsj%2Ft9803_use_path_helper_fn-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1691/sanchit1053/sj/t9803_use_path_helper_fn-v1
Pull-Request: https://github.com/git/git/pull/1691
-- 
gitgitgadget
sanchit1053 via GitGitGadget· Mar 21, 2024, 19:39 UTC · re: Sanchit Jindal via GitGitGadget · lore

[PATCH 1/2] t9803-git-p4-shell-metachars.sh: update to use test_path_* functions

From: sanchit1053 <sanchit1053@gmail.com>
Signed-off-by: sanchit1053 <sanchit1053@gmail.com>
---
 t/t9803-git-p4-shell-metachars.sh | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
Show changes to t/t9803-git-p4-shell-metachars.sh +6 −6
diff --git a/t/t9803-git-p4-shell-metachars.sh b/t/t9803-git-p4-shell-metachars.sh
index 2913277013d..4905ed2ae9e 100755
--- a/t/t9803-git-p4-shell-metachars.sh
+++ b/t/t9803-git-p4-shell-metachars.sh
@@ -33,8 +33,8 @@ test_expect_success 'shell metachars in filenames' '
 	(
 		cd "$cli" &&
 		p4 sync ... &&
-		test -e "file with spaces" &&
-		test -e "foo\$bar"
+		test_path_exists "file with spaces" &&
+		test_path_exists "foo\$bar"
 	)
 '
 
@@ -52,8 +52,8 @@ test_expect_success 'deleting with shell metachars' '
 	(
 		cd "$cli" &&
 		p4 sync ... &&
-		test ! -e "file with spaces" &&
-		test ! -e foo\$bar
+		test_path_is_missing "file with spaces" &&
+		test_path_is_missing foo\$bar
 	)
 '
 
@@ -100,8 +100,8 @@ test_expect_success 'branch with shell char' '
 		git p4 clone --dest=. --detect-branches //depot@all &&
 		git log --all --graph --decorate --stat &&
 		git reset --hard p4/depot/branch\$3 &&
-		test -f shell_char_branch_file &&
-		test -f f1
+		test_path_is_file shell_char_branch_file &&
+		test_path_is_file f1
 	)
 '
 
-- 
gitgitgadget
Sanchit Jindal via GitGitGadget· Mar 21, 2024, 19:39 UTC · re: Sanchit Jindal via GitGitGadget · lore

[PATCH 2/2] t9803: update commit messages and description

From: Sanchit Jindal <sanchit1053@gmail.com>
replacing `test -e` with test_path_exists,
	  `test ! -e` with test_path_is_missing
	  `test -f` with test_path_is_file
These helper functions will run the `test` command with the
corresponding flags and will echo a message if the assert fails.
This will provide better debugging logs for test, instead of the
previous method which provided no message
Signed-off-by: Sanchit Jindal <sanchit1053@gmail.com>
-- 
gitgitgadget
Eric Sunshine· Mar 21, 2024, 19:52 UTC · re: Sanchit Jindal via GitGitGadget · lore

Re: [PATCH 2/2] t9803: update commit messages and description

On Thu, Mar 21, 2024 at 3:39 PM Sanchit Jindal via GitGitGadget <gitgitgadget@gmail.com> wrote:

Show 11 quoted lines
> From: Sanchit Jindal <sanchit1053@gmail.com>
>
> replacing `test -e` with test_path_exists,
>           `test ! -e` with test_path_is_missing
>           `test -f` with test_path_is_file
> These helper functions will run the `test` command with the
> corresponding flags and will echo a message if the assert fails.
> This will provide better debugging logs for test, instead of the
> previous method which provided no message
>
> Signed-off-by: Sanchit Jindal <sanchit1053@gmail.com>

When rerolling a series to address reviewer comments, you will want the fixes applied directly to the patches about which the reviewers commented. The way to do this is to use `git rebase -i` to adjust the patches as needed. In the case of this simple series, you just want to "squash" patches [1/2] and [2/2] into a single patch using the `git rebase -i` "squash" command, and adjust the commit message of the squashed patch appropriately. Finally, to resubmit it via GitGitGadget, force-push the revised series to GitGitGadget (using `git push --force <whatever> <whatever>`), and tell GitGitGadget to "/submit".

Regarding the commit message, first explain the problem the patch is solving, and then explain how the patch solves it. Thus, start by explaining that `test` doesn't provide any diagnostic information when it fails, which isn't helpful to test authors. Then explain that the patch replaces `test` with the test_path_* functions which do provide useful diagnostic information.

The From: and Signed-off-by: lines look good in this reroll.
Sanchit Jindal· Mar 21, 2024, 20:28 UTC · re: Eric Sunshine · lore

Re: [PATCH 2/2] t9803: update commit messages and description

On Fri, 22 Mar 2024 at 01:22, Eric Sunshine <sunshine@sunshineco.com> wrote:
Show 34 quoted lines
>
> On Thu, Mar 21, 2024 at 3:39 PM Sanchit Jindal via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
> > From: Sanchit Jindal <sanchit1053@gmail.com>
> >
> > replacing `test -e` with test_path_exists,
> >           `test ! -e` with test_path_is_missing
> >           `test -f` with test_path_is_file
> > These helper functions will run the `test` command with the
> > corresponding flags and will echo a message if the assert fails.
> > This will provide better debugging logs for test, instead of the
> > previous method which provided no message
> >
> > Signed-off-by: Sanchit Jindal <sanchit1053@gmail.com>
>
> When rerolling a series to address reviewer comments, you will want
> the fixes applied directly to the patches about which the reviewers
> commented. The way to do this is to use `git rebase -i` to adjust the
> patches as needed. In the case of this simple series, you just want to
> "squash" patches [1/2] and [2/2] into a single patch using the `git
> rebase -i` "squash" command, and adjust the commit message of the
> squashed patch appropriately. Finally, to resubmit it via
> GitGitGadget, force-push the revised series to GitGitGadget (using
> `git push --force <whatever> <whatever>`), and tell GitGitGadget to
> "/submit".
>
> Regarding the commit message, first explain the problem the patch is
> solving, and then explain how the patch solves it. Thus, start by
> explaining that `test` doesn't provide any diagnostic information when
> it fails, which isn't helpful to test authors. Then explain that the
> patch replaces `test` with the test_path_* functions which do provide
> useful diagnostic information.
>
> The From: and Signed-off-by: lines look good in this reroll.
Thank You for the review

I renamed the branch using github as it had a typo which made the earlier patch to be erased.I am extremely sorry about this.

I have created another patch with a more descriptive commit message, and after squashing it with the earlier commit. I hope it is satisfactory

I wanted to ask, Is it possible to send the patches created using `git format-patch` manually using gmail, Or the default headers applied by `send-email` required.

Regards, Sanchit Jindal

Junio C Hamano· Mar 21, 2024, 23:25 UTC · re: Sanchit Jindal · lore

Re: [PATCH 2/2] t9803: update commit messages and description

Sanchit Jindal <sanchit1053@gmail.com> writes:
> I have created another patch with a more descriptive commit message,
> and after squashing it with the earlier commit. I hope it is satisfactory

Hopefully. What GGG sent with v2 label still was a 2-patch series, whose [2/2] patch was an empty patch, though.

> I wanted to ask, Is it possible to send the patches created using
> `git format-patch` manually using gmail, Or the default headers
> applied by `send-email` required.

I do not know where you are using gmail from (I know that Android version has no way to send text-only e-mail for example), but when cutting and pasting the output from format-patch, you should make sure

 * Remove "From <commit object name> Mon Sep 17 00:00:00 2001"
   marker line that signals the beginning of each patch message.  It
   is a mistake to leave this line in the body of your message.
 * Remove "Date: " header.  Do not leave this line in the body of
   your message.
 * Remove "Subject: " header and move its contents to your MUA's
   Subject entry field.
 * Remove "From: " header, and arrange that your MUA puts the same
   "Sanchit Jindal <sanchit1053@gmail.com> on the From header.  If
   your MUA is not cooperating, you can leave that line in the body
   of the message
 * Your cut&paste does not corrupt whitespaces, like squashing two
   consecutive spaces into one, removing the leading whitespaces,
   turning a tab into a run of spaces, folding a line at a
   whitespace in the middle of the message, etc.
 * Your MUA does not turn your text message into HTML gunk.

As long as you do the above carefully, you should be OK. "git format-patch --help" has a section on MUA specific hints, which also might be helpful.

Thanks.
Sanchit Jindal via GitGitGadget· Mar 21, 2024, 20:29 UTC · re: Sanchit Jindal via GitGitGadget · lore

[PATCH v2 0/2] t9803-git-p4-shell-metachars.sh: update to use test_path_* functions

Correct Typo in Branch name
Sanchit Jindal (1):
  t9803: update commit messages and description
sanchit1053 (1):
  t9803-git-p4-shell-metachars.sh: update to use test_path_* functions
 t/t9803-git-p4-shell-metachars.sh | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
base-commit: 3bd955d26919e149552f34aacf8a4e6368c26cec
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1691%2Fsanchit1053%2Fsj%2Ft9803_use_path_helper_fn-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1691/sanchit1053/sj/t9803_use_path_helper_fn-v2
Pull-Request: https://github.com/git/git/pull/1691
Range-diff vs v1:
 1:  b541ce7a49c = 1:  b541ce7a49c t9803-git-p4-shell-metachars.sh: update to use test_path_* functions
 2:  b8d0620d410 < -:  ----------- t9803: update commit messages and description
 -:  ----------- > 2:  e25b5f928a9 t9803: update commit messages and description
-- 
gitgitgadget
sanchit1053 via GitGitGadget· Mar 21, 2024, 20:29 UTC · re: Sanchit Jindal via GitGitGadget · lore

[PATCH v2 1/2] t9803-git-p4-shell-metachars.sh: update to use test_path_* functions

From: sanchit1053 <sanchit1053@gmail.com>
Signed-off-by: sanchit1053 <sanchit1053@gmail.com>
---
 t/t9803-git-p4-shell-metachars.sh | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
Show changes to t/t9803-git-p4-shell-metachars.sh +6 −6
diff --git a/t/t9803-git-p4-shell-metachars.sh b/t/t9803-git-p4-shell-metachars.sh
index 2913277013d..4905ed2ae9e 100755
--- a/t/t9803-git-p4-shell-metachars.sh
+++ b/t/t9803-git-p4-shell-metachars.sh
@@ -33,8 +33,8 @@ test_expect_success 'shell metachars in filenames' '
 	(
 		cd "$cli" &&
 		p4 sync ... &&
-		test -e "file with spaces" &&
-		test -e "foo\$bar"
+		test_path_exists "file with spaces" &&
+		test_path_exists "foo\$bar"
 	)
 '
 
@@ -52,8 +52,8 @@ test_expect_success 'deleting with shell metachars' '
 	(
 		cd "$cli" &&
 		p4 sync ... &&
-		test ! -e "file with spaces" &&
-		test ! -e foo\$bar
+		test_path_is_missing "file with spaces" &&
+		test_path_is_missing foo\$bar
 	)
 '
 
@@ -100,8 +100,8 @@ test_expect_success 'branch with shell char' '
 		git p4 clone --dest=. --detect-branches //depot@all &&
 		git log --all --graph --decorate --stat &&
 		git reset --hard p4/depot/branch\$3 &&
-		test -f shell_char_branch_file &&
-		test -f f1
+		test_path_is_file shell_char_branch_file &&
+		test_path_is_file f1
 	)
 '
 
-- 
gitgitgadget
Sanchit Jindal via GitGitGadget· Mar 21, 2024, 20:29 UTC · re: Sanchit Jindal via GitGitGadget · lore

[PATCH v2 2/2] t9803: update commit messages and description

From: Sanchit Jindal <sanchit1053@gmail.com>

Using the `test` function directly does not provide any diagnostic information to the user, in case of a failure.

This patch replace the `test` function with the helper functions defined that will call the corresponding test function and log an error message if the assert fails

Making the replacements
- `test -e` -> test_path_exists,
- `test ! -e` -> test_path_is_missing
- `test -f` -> test_path_is_file
Signed-off-by: Sanchit Jindal <sanchit1053@gmail.com>
-- 
gitgitgadget

← back to recent threads