* [PATCH] t4200-rerere.sh: use test_path_is_* functions in the script
@ 2020-03-30 17:16 Allo via GitGitGadget
2020-04-09 14:54 ` [PATCH v2] " Allo via GitGitGadget
0 siblings, 1 reply; 4+ messages in thread
From: Allo via GitGitGadget @ 2020-03-30 17:16 UTC (permalink / raw)
To: git; +Cc: Allo, Angel Pan
From: Angel Pan <dinoallosaurus1111@gmail.com>
This is a microproject for GSoC.
The original code works fine. However, since the code base has already have help-functions(in test-lib-functions),
the change is able to help people understand the code better, as well as improve debug-friendliness.
Signed-off-by: Angel Pan <dinoallosaurus1111@gmail.com>
---
[GSoC/PATCH] t4200-rerere.sh: use test_path_is_* functions in the script
Hello again. I have sent a mail here: last mail
[https://lore.kernel.org/git/CAFk=nY7efz8ZisEuyAcU09J+ja4qE8LorUUPeupGVEjBBc6HDA@mail.gmail.com/T/#u]
After all, I decided to replace test -(e|f|d) with test_path_is_*
functions as my microproject for GSoC. I am looking forward to working
with every contributor here. I also have another question: For a commit
like this involving 10+ insertions, would it be better if I
seperate-commited all of them? But this kind of change seems trivial...
Signed-off-by: Angel Pan dinoallosaurus1111@gmail.com
[dinoallosaurus1111@gmail.com]
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-595%2Fdinoallo%2Fmaster-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-595/dinoallo/master-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/595
t/t4200-rerere.sh | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh
index 831d424c472..da8be1c6254 100755
--- a/t/t4200-rerere.sh
+++ b/t/t4200-rerere.sh
@@ -70,7 +70,7 @@ test_expect_success 'nothing recorded without rerere' '
rm -rf .git/rr-cache &&
git config rerere.enabled false &&
test_must_fail git merge first &&
- ! test -d .git/rr-cache
+ ! test_path_is_dir .git/rr-cache
'
test_expect_success 'activate rerere, old style (conflicting merge)' '
@@ -82,8 +82,8 @@ test_expect_success 'activate rerere, old style (conflicting merge)' '
sha1=$(perl -pe "s/ .*//" .git/MERGE_RR) &&
rr=.git/rr-cache/$sha1 &&
grep "^=======\$" $rr/preimage &&
- ! test -f $rr/postimage &&
- ! test -f $rr/thisimage
+ ! test_path_is_file $rr/postimage &&
+ ! test_path_is_file $rr/thisimage
'
test_expect_success 'rerere.enabled works, too' '
@@ -108,8 +108,8 @@ test_expect_success 'set up rr-cache' '
test_expect_success 'rr-cache looks sane' '
# no postimage or thisimage yet
- ! test -f $rr/postimage &&
- ! test -f $rr/thisimage &&
+ ! test_path_is_file $rr/postimage &&
+ ! test_path_is_file $rr/thisimage &&
# preimage has right number of lines
cnt=$(sed -ne "/^<<<<<<</,/^>>>>>>>/p" $rr/preimage | wc -l) &&
@@ -165,7 +165,7 @@ test_expect_success 'first postimage wins' '
git show first:a1 | sed "s/To die: t/To die! T/" >expect &&
git commit -q -a -m "prefer first over second" &&
- test -f $rr/postimage &&
+ test_path_is_file $rr/postimage &&
oldmtimepost=$(test-tool chmtime --get -60 $rr/postimage) &&
@@ -188,14 +188,14 @@ test_expect_success 'rerere clear' '
mv $rr/postimage .git/post-saved &&
echo "$sha1 a1" | perl -pe "y/\012/\000/" >.git/MERGE_RR &&
git rerere clear &&
- ! test -d $rr
+ ! test_path_is_dir $rr
'
test_expect_success 'leftover directory' '
git reset --hard &&
mkdir -p $rr &&
test_must_fail git merge first &&
- test -f $rr/preimage
+ test_path_is_file $rr/preimage
'
test_expect_success 'missing preimage' '
@@ -203,7 +203,7 @@ test_expect_success 'missing preimage' '
mkdir -p $rr &&
cp .git/post-saved $rr/postimage &&
test_must_fail git merge first &&
- test -f $rr/preimage
+ test_path_is_file $rr/preimage
'
test_expect_success 'set up for garbage collection tests' '
@@ -228,16 +228,16 @@ test_expect_success 'set up for garbage collection tests' '
test_expect_success 'gc preserves young or recently used records' '
git rerere gc &&
- test -f $rr/preimage &&
- test -f $rr2/preimage
+ test_path_is_file $rr/preimage &&
+ test_path_is_file $rr2/preimage
'
test_expect_success 'old records rest in peace' '
test-tool chmtime =$just_over_60_days_ago $rr/postimage &&
test-tool chmtime =$just_over_15_days_ago $rr2/preimage &&
git rerere gc &&
- ! test -f $rr/preimage &&
- ! test -f $rr2/preimage
+ ! test_path_is_file $rr/preimage &&
+ ! test_path_is_file $rr2/preimage
'
rerere_gc_custom_expiry_test () {
base-commit: 9fadedd637b312089337d73c3ed8447e9f0aa775
--
gitgitgadget
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH v2] t4200-rerere.sh: use test_path_is_* functions in the script
2020-03-30 17:16 [PATCH] t4200-rerere.sh: use test_path_is_* functions in the script Allo via GitGitGadget
@ 2020-04-09 14:54 ` Allo via GitGitGadget
2020-04-09 15:21 ` Danh Doan
0 siblings, 1 reply; 4+ messages in thread
From: Allo via GitGitGadget @ 2020-04-09 14:54 UTC (permalink / raw)
To: git; +Cc: Allo, Angel Pan
From: Angel Pan <dinoallosaurus1111@gmail.com>
The original code works fine. However, since the code base has already had help-functions(in test-lib-functions),
the change is able to help people understand the code better, as well as improve debug-friendliness.
* t4200-rerere.sh: replace "test -d" with "test_path_is_dir"
* t4200-rerere.sh: replace "test -f" with "test_path_is_file"
Signed-off-by: Angel Pan <dinoallosaurus1111@gmail.com>
---
[GSoC/PATCH] t4200-rerere.sh: use test_path_is_* functions in the script
Hello again. I thought I needed to improve my commit message since some
of community members friendly reminded me last week. So I revised it
again. Here is my patch with new commit message. If there is still
something I miss, a reminder will be appreciated.
Signed-off-by: Angel Pan dinoallosaurus1111@gmail.com
[dinoallosaurus1111@gmail.com]
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-595%2Fdinoallo%2Fmaster-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-595/dinoallo/master-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/595
Range-diff vs v1:
1: 7e0c8749016 ! 1: c704627a465 t4200-rerere.sh: use test_path_is_* functions in the script
@@ Metadata
## Commit message ##
t4200-rerere.sh: use test_path_is_* functions in the script
- This is a microproject for GSoC.
- The original code works fine. However, since the code base has already have help-functions(in test-lib-functions),
+ The original code works fine. However, since the code base has already had help-functions(in test-lib-functions),
the change is able to help people understand the code better, as well as improve debug-friendliness.
+ * t4200-rerere.sh: replace "test -d" with "test_path_is_dir"
+ * t4200-rerere.sh: replace "test -f" with "test_path_is_file"
+
Signed-off-by: Angel Pan <dinoallosaurus1111@gmail.com>
## t/t4200-rerere.sh ##
t/t4200-rerere.sh | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh
index 831d424c472..da8be1c6254 100755
--- a/t/t4200-rerere.sh
+++ b/t/t4200-rerere.sh
@@ -70,7 +70,7 @@ test_expect_success 'nothing recorded without rerere' '
rm -rf .git/rr-cache &&
git config rerere.enabled false &&
test_must_fail git merge first &&
- ! test -d .git/rr-cache
+ ! test_path_is_dir .git/rr-cache
'
test_expect_success 'activate rerere, old style (conflicting merge)' '
@@ -82,8 +82,8 @@ test_expect_success 'activate rerere, old style (conflicting merge)' '
sha1=$(perl -pe "s/ .*//" .git/MERGE_RR) &&
rr=.git/rr-cache/$sha1 &&
grep "^=======\$" $rr/preimage &&
- ! test -f $rr/postimage &&
- ! test -f $rr/thisimage
+ ! test_path_is_file $rr/postimage &&
+ ! test_path_is_file $rr/thisimage
'
test_expect_success 'rerere.enabled works, too' '
@@ -108,8 +108,8 @@ test_expect_success 'set up rr-cache' '
test_expect_success 'rr-cache looks sane' '
# no postimage or thisimage yet
- ! test -f $rr/postimage &&
- ! test -f $rr/thisimage &&
+ ! test_path_is_file $rr/postimage &&
+ ! test_path_is_file $rr/thisimage &&
# preimage has right number of lines
cnt=$(sed -ne "/^<<<<<<</,/^>>>>>>>/p" $rr/preimage | wc -l) &&
@@ -165,7 +165,7 @@ test_expect_success 'first postimage wins' '
git show first:a1 | sed "s/To die: t/To die! T/" >expect &&
git commit -q -a -m "prefer first over second" &&
- test -f $rr/postimage &&
+ test_path_is_file $rr/postimage &&
oldmtimepost=$(test-tool chmtime --get -60 $rr/postimage) &&
@@ -188,14 +188,14 @@ test_expect_success 'rerere clear' '
mv $rr/postimage .git/post-saved &&
echo "$sha1 a1" | perl -pe "y/\012/\000/" >.git/MERGE_RR &&
git rerere clear &&
- ! test -d $rr
+ ! test_path_is_dir $rr
'
test_expect_success 'leftover directory' '
git reset --hard &&
mkdir -p $rr &&
test_must_fail git merge first &&
- test -f $rr/preimage
+ test_path_is_file $rr/preimage
'
test_expect_success 'missing preimage' '
@@ -203,7 +203,7 @@ test_expect_success 'missing preimage' '
mkdir -p $rr &&
cp .git/post-saved $rr/postimage &&
test_must_fail git merge first &&
- test -f $rr/preimage
+ test_path_is_file $rr/preimage
'
test_expect_success 'set up for garbage collection tests' '
@@ -228,16 +228,16 @@ test_expect_success 'set up for garbage collection tests' '
test_expect_success 'gc preserves young or recently used records' '
git rerere gc &&
- test -f $rr/preimage &&
- test -f $rr2/preimage
+ test_path_is_file $rr/preimage &&
+ test_path_is_file $rr2/preimage
'
test_expect_success 'old records rest in peace' '
test-tool chmtime =$just_over_60_days_ago $rr/postimage &&
test-tool chmtime =$just_over_15_days_ago $rr2/preimage &&
git rerere gc &&
- ! test -f $rr/preimage &&
- ! test -f $rr2/preimage
+ ! test_path_is_file $rr/preimage &&
+ ! test_path_is_file $rr2/preimage
'
rerere_gc_custom_expiry_test () {
base-commit: 9fadedd637b312089337d73c3ed8447e9f0aa775
--
gitgitgadget
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] t4200-rerere.sh: use test_path_is_* functions in the script
2020-04-09 14:54 ` [PATCH v2] " Allo via GitGitGadget
@ 2020-04-09 15:21 ` Danh Doan
2020-04-09 18:09 ` Junio C Hamano
0 siblings, 1 reply; 4+ messages in thread
From: Danh Doan @ 2020-04-09 15:21 UTC (permalink / raw)
To: Allo via GitGitGadget; +Cc: git, Allo
On 2020-04-09 14:54:21+0000, Allo via GitGitGadget <gitgitgadget@gmail.com> wrote:
> From: Angel Pan <dinoallosaurus1111@gmail.com>
>
> Subject: t4200-rerere.sh: use test_path_is_* functions in the script
"in the script" looks too verbose to me. Because the script has been
mentioned earlier.
> The original code works fine. However, since the code base has already had help-functions(in test-lib-functions),
We wrap lines at 72 characters per line.
And, missing space before open parenthese.
I'm not good at wording, but I think this commit message is too
verbose. I would say something like:
Prefer "test_path_is_dir" to "test -d" and test_path_is_file
to "test -f" because they're more friendly to debug.
That's not a good message, either. But, it's some what simpler and
cleaner.
> the change is able to help people understand the code better, as well as improve debug-friendliness.
>
> * t4200-rerere.sh: replace "test -d" with "test_path_is_dir"
> * t4200-rerere.sh: replace "test -f" with "test_path_is_file"
>
--
Danh
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] t4200-rerere.sh: use test_path_is_* functions in the script
2020-04-09 15:21 ` Danh Doan
@ 2020-04-09 18:09 ` Junio C Hamano
0 siblings, 0 replies; 4+ messages in thread
From: Junio C Hamano @ 2020-04-09 18:09 UTC (permalink / raw)
To: Danh Doan; +Cc: Allo via GitGitGadget, git, Allo
Danh Doan <congdanhqx@gmail.com> writes:
> On 2020-04-09 14:54:21+0000, Allo via GitGitGadget <gitgitgadget@gmail.com> wrote:
>> From: Angel Pan <dinoallosaurus1111@gmail.com>
>> ...
> That's not a good message, either. But, it's some what simpler and
> cleaner.
Thank you, Allo, for a patch, and thank you Danh for a good review.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-04-09 18:09 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-30 17:16 [PATCH] t4200-rerere.sh: use test_path_is_* functions in the script Allo via GitGitGadget
2020-04-09 14:54 ` [PATCH v2] " Allo via GitGitGadget
2020-04-09 15:21 ` Danh Doan
2020-04-09 18:09 ` Junio C Hamano
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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 read-only IMAP folder(s) and NNTP newsgroup(s).