diff mbox series

[GSoC,2/2] t4131: use helper function to replace test -f <path>

Message ID 20200319132957.17813-3-harshitjain1371999@gmail.com (mailing list archive)
State New, archived
Headers show
Series t4131: update test script | expand

Commit Message

Harshit Jain March 19, 2020, 1:29 p.m. UTC
Replace 'test -f' with the helper function 'test_path_is_file' as the helper function improves the code readability and also gives better error messages.

Signed-off-by: Harshit Jain <harshitjain1371999@gmail.com>
---
 t/t4131-apply-fake-ancestor.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Shourya Shukla March 19, 2020, 4:42 p.m. UTC | #1
Hello Harshit,

> Replace 'test -f' with the helper function 'test_path_is_file' as the helper function improves the code readability and also gives better error messages.

Again the same thing, you may follow what I stated before regarding commit messages.

The commit title can be of the form:

t4131: use helpers to replace test -f <path>

<<commit description>>

If you still face any problem, feel free to drop a message :)

Regards,
Shourya Shukla
Kaartic Sivaraam March 19, 2020, 5:33 p.m. UTC | #2
On 19-03-2020 22:12, Shourya Shukla wrote:
> Hello Harshit,
> 
>> Replace 'test -f' with the helper function 'test_path_is_file' as the helper function improves the code readability and also gives better error messages.
> 
> Again the same thing, you may follow what I stated before regarding commit messages.
> 
> The commit title can be of the form:
> 
> t4131: use helpers to replace test -f <path>
> 
> <<commit description>>
> 

Just curious, isn't the commit title already like that in this patch? 
The subject does read:

   [GSoC][PATCH 2/2] t4131: use helper function to replace test -f <path>"

What am I missing?
Harshit Jain March 19, 2020, 8:18 p.m. UTC | #3
On Thu, Mar 19, 2020 at 11:04 PM Kaartic Sivaraam
<kaartic.sivaraam@gmail.com> wrote:
>
> On 19-03-2020 22:12, Shourya Shukla wrote:
> > Hello Harshit,
> >
> >> Replace 'test -f' with the helper function 'test_path_is_file' as the helper function improves the code readability and also gives better error messages.
> >
> > Again the same thing, you may follow what I stated before regarding commit messages.
> >
> > The commit title can be of the form:
> >
> > t4131: use helpers to replace test -f <path>
> >
> > <<commit description>>
> >
>
> Just curious, isn't the commit title already like that in this patch?
> The subject does read:
>
>    [GSoC][PATCH 2/2] t4131: use helper function to replace test -f <path>"
>
> What am I missing?
>

Hey Shourya,
Can you please clarify, I am also a bit confused.

Regards,
Harshit Jain
Junio C Hamano March 19, 2020, 9:58 p.m. UTC | #4
Shourya Shukla <shouryashukla.oo@gmail.com> writes:

> Hello Harshit,
>
>> Replace 'test -f' with the helper function 'test_path_is_file' as the helper function improves the code readability and also gives better error messages.
>
> Again the same thing, you may follow what I stated before regarding commit messages.
>
> The commit title can be of the form:
>
> t4131: use helpers to replace test -f <path>
>
> <<commit description>>

I think Harshit is writing the title of the commit in the right
place.  Format-wise, the only thing that is wrong is that each
paragraph is too long without line wrapping.

What is wrong in these two e-mail thread is that you are not reading
the log message correctly.  When made into a piece of e-mail, the
title goes to the "Subject:" field in the header and there is no
need to repeat it in the body of the e-mail.
Shourya Shukla March 20, 2020, 3:39 p.m. UTC | #5
Hello Kaartic,

Apologies, I totally missed out on that.

Regards,
Shourya Shukla
Shourya Shukla March 20, 2020, 3:43 p.m. UTC | #6
Apologies, I totally missed that out for some reason. Silly me! :/
diff mbox series

Patch

diff --git a/t/t4131-apply-fake-ancestor.sh b/t/t4131-apply-fake-ancestor.sh
index 828d1a355b..21ee359632 100755
--- a/t/t4131-apply-fake-ancestor.sh
+++ b/t/t4131-apply-fake-ancestor.sh
@@ -33,7 +33,7 @@  test_expect_success 'apply --build-fake-ancestor in a subdirectory' '
 	(
 		cd sub &&
 		git apply --build-fake-ancestor 3.ancestor ../3.patch &&
-		test -f 3.ancestor
+		test_path_is_file 3.ancestor
 	) &&
 	git apply --build-fake-ancestor 3.ancestor 3.patch &&
 	test_cmp sub/3.ancestor 3.ancestor