{"thread":{"id":"58625","subject":"[PATCH] [OUTREACHY] t1002: modernize outdated conditional","startedAt":"2022-10-14T02:06:34Z","lastAt":"2022-10-14T20:43:03Z","messageCount":15,"participants":["nsengaw4c via GitGitGadget","Junio C Hamano","Derrick Stolee","Eric Sunshine","Philip Oakley"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"464905","messageId":"pull.1362.git.git.1665713184304.gitgitgadget@gmail.com","threadId":"58625","inReplyTo":null,"subject":"[PATCH] [OUTREACHY] t1002: modernize outdated conditional","fromName":"nsengaw4c via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-10-14T02:06:24Z","receivedAt":"2022-10-14T02:06:34Z","isPatch":true,"sender":{"key":"name:nsengaw4c","avatar":null},"body":"From: wilberforce \n\nTests in this script use an unusual and hard to reason about\nconditional construct\n\n if expression; then false; else :; fi\n\nChange them to use more idiomatic construct:\n\n ! expression\n\nCc: Christian Couder \nCc: Hariom Verma \nSigned-off-by: Nsengiyumva wilberfore \n---\n [OUTREACHY]cleaning t1002-read-tree-m-u-2way.sh\n \n This is an update in t1002-read-tree-m-u-2way.sh. all the tests that use\n the unusual construct: if read_tree_u_must_succeed -m -u $treeH $treeM;\n then false; else :; fi have been updated to ! read_tree_u_must_succeed\n -m -u $treeH $treeM \"I am an outreachy applicant\" CC: Christian Couder\n christian.couder@gmail.com, Hariom verma hariom18599@gmail.com\n Signed-off-by: wilberforce nsengiyumvawilberforce@gmail.com\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1362%2Fnsengiyumva-wilberforce%2Ft1002_usual_construct_updated-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1362/nsengiyumva-wilberforce/t1002_usual_construct_updated-v1\nPull-Request: https://github.com/git/git/pull/1362\n\n t/t1002-read-tree-m-u-2way.sh | 16 ++++++++--------\n 1 file changed, 8 insertions(+), 8 deletions(-)\n\ndiff --git a/t/t1002-read-tree-m-u-2way.sh b/t/t1002-read-tree-m-u-2way.sh\nindex bd5313caec9..cdc077ce12d 100755\n--- a/t/t1002-read-tree-m-u-2way.sh\n+++ b/t/t1002-read-tree-m-u-2way.sh\n@@ -154,7 +154,7 @@ test_expect_success \\\n read_tree_u_must_succeed --reset -u $treeH &&\n echo frotz frotz >frotz &&\n git update-index --add frotz &&\n- if read_tree_u_must_succeed -m -u $treeH $treeM; then false; else :; fi'\n+ ! read_tree_u_must_succeed -m -u $treeH $treeM'\n \n test_expect_success \\\n '9 - conflicting addition.' \\\n@@ -163,7 +163,7 @@ test_expect_success \\\n echo frotz frotz >frotz &&\n git update-index --add frotz &&\n echo frotz >frotz &&\n- if read_tree_u_must_succeed -m -u $treeH $treeM; then false; else :; fi'\n+ ! read_tree_u_must_succeed -m -u $treeH $treeM'\n \n test_expect_success \\\n '10 - path removed.' \\\n@@ -186,7 +186,7 @@ test_expect_success \\\n echo rezrov >rezrov &&\n git update-index --add rezrov &&\n echo rezrov rezrov >rezrov &&\n- if read_tree_u_must_succeed -m -u $treeH $treeM; then false; else :; fi'\n+ ! read_tree_u_must_succeed -m -u $treeH $treeM'\n \n test_expect_success \\\n '12 - unmatching local changes being removed.' \\\n@@ -194,7 +194,7 @@ test_expect_success \\\n read_tree_u_must_succeed --reset -u $treeH &&\n echo rezrov rezrov >rezrov &&\n git update-index --add rezrov &&\n- if read_tree_u_must_succeed -m -u $treeH $treeM; then false; else :; fi'\n+ ! read_tree_u_must_succeed -m -u $treeH $treeM'\n \n test_expect_success \\\n '13 - unmatching local changes being removed.' \\\n@@ -203,7 +203,7 @@ test_expect_success \\\n echo rezrov rezrov >rezrov &&\n git update-index --add rezrov &&\n echo rezrov >rezrov &&\n- if read_tree_u_must_succeed -m -u $treeH $treeM; then false; else :; fi'\n+ ! read_tree_u_must_succeed -m -u $treeH $treeM'\n \n cat >expected <bozbar &&\n git update-index --add bozbar &&\n- if read_tree_u_must_succeed -m -u $treeH $treeM; then false; else :; fi'\n+ ! read_tree_u_must_succeed -m -u $treeH $treeM'\n \n test_expect_success \\\n '17 - conflicting local change.' \\\n@@ -260,7 +260,7 @@ test_expect_success \\\n echo bozbar bozbar >bozbar &&\n git update-index --add bozbar &&\n echo bozbar bozbar bozbar >bozbar &&\n- if read_tree_u_must_succeed -m -u $treeH $treeM; then false; else :; fi'\n+ ! read_tree_u_must_succeed -m -u $treeH $treeM'\n \n test_expect_success \\\n '18 - local change already having a good result.' \\\n@@ -316,7 +316,7 @@ test_expect_success \\\n echo bozbar >bozbar &&\n git update-index --add bozbar &&\n echo gnusto gnusto >bozbar &&\n- if read_tree_u_must_succeed -m -u $treeH $treeM; then false; else :; fi'\n+ ! read_tree_u_must_succeed -m -u $treeH $treeM'\n \n # Also make sure we did not break DF vs DF/DF case.\n test_expect_success \\\n\nbase-commit: d420dda0576340909c3faff364cfbd1485f70376\n-- \ngitgitgadget\n"},{"id":"464908","messageId":"xmqqy1tjatai.fsf@gitster.g","threadId":"58625","inReplyTo":"pull.1362.git.git.1665713184304.gitgitgadget@gmail.com","subject":"Re: [PATCH] [OUTREACHY] t1002: modernize outdated conditional","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-10-14T05:05:57Z","receivedAt":"2022-10-14T05:06:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"nsengaw4c via GitGitGadget\" writes:\n\n> From: wilberforce \n\nThis name must match ...\n\n> Tests in this script use an unusual and hard to reason about\n> conditional construct\n>\n> if expression; then false; else :; fi\n>\n> Change them to use more idiomatic construct:\n>\n> ! expression\n>\n> Cc: Christian Couder \n> Cc: Hariom Verma \n> Signed-off-by: Nsengiyumva wilberfore \n\n... the name you sign-off your work with.\n\n> - if read_tree_u_must_succeed -m -u $treeH $treeM; then false; else :; fi'\n> + ! read_tree_u_must_succeed -m -u $treeH $treeM'\n\nOK.\n\n"},{"id":"464910","messageId":"pull.1362.v2.git.git.1665733647421.gitgitgadget@gmail.com","threadId":"58625","inReplyTo":"pull.1362.git.git.1665713184304.gitgitgadget@gmail.com","subject":"[PATCH v2] [OUTREACHY] t1002: modernize outdated conditional","fromName":"nsengaw4c via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-10-14T07:47:27Z","receivedAt":"2022-10-14T07:47:36Z","isPatch":true,"sender":{"key":"name:nsengaw4c","avatar":null},"body":"From: Nsengiyumva Wilberforce \n\nTests in this script use an unusual and hard to reason about\nconditional construct\n\n if expression; then false; else :; fi\n\nChange them to use more idiomatic construct:\n\n ! expression\n\nCc: Christian Couder \nCc: Hariom Verma \nSigned-off-by: Nsengiyumva Wilberfore \n---\n [OUTREACHY]cleaning t1002-read-tree-m-u-2way.sh\n \n This is an update in t1002-read-tree-m-u-2way.sh. all the tests that use\n the unusual construct: if read_tree_u_must_succeed -m -u $treeH $treeM;\n then false; else :; fi have been updated to ! read_tree_u_must_succeed\n -m -u $treeH $treeM \"I am an outreachy applicant\" CC: Christian Couder\n christian.couder@gmail.com, Hariom verma hariom18599@gmail.com\n Signed-off-by: wilberforce nsengiyumvawilberforce@gmail.com\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1362%2Fnsengiyumva-wilberforce%2Ft1002_usual_construct_updated-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1362/nsengiyumva-wilberforce/t1002_usual_construct_updated-v2\nPull-Request: https://github.com/git/git/pull/1362\n\nRange-diff vs v1:\n\n 1: b2ed686bc94 ! 1: 8a9cd66d7d9 [OUTREACHY] t1002: modernize outdated conditional\n @@\n ## Metadata ##\n -Author: wilberforce \n +Author: Nsengiyumva Wilberforce \n \n ## Commit message ##\n [OUTREACHY] t1002: modernize outdated conditional\n @@ Commit message\n \n Cc: Christian Couder \n Cc: Hariom Verma \n - Signed-off-by: Nsengiyumva wilberfore \n + Signed-off-by: Nsengiyumva Wilberfore \n \n ## t/t1002-read-tree-m-u-2way.sh ##\n @@ t/t1002-read-tree-m-u-2way.sh: test_expect_success \\\n\n\n t/t1002-read-tree-m-u-2way.sh | 16 ++++++++--------\n 1 file changed, 8 insertions(+), 8 deletions(-)\n\ndiff --git a/t/t1002-read-tree-m-u-2way.sh b/t/t1002-read-tree-m-u-2way.sh\nindex bd5313caec9..cdc077ce12d 100755\n--- a/t/t1002-read-tree-m-u-2way.sh\n+++ b/t/t1002-read-tree-m-u-2way.sh\n@@ -154,7 +154,7 @@ test_expect_success \\\n read_tree_u_must_succeed --reset -u $treeH &&\n echo frotz frotz >frotz &&\n git update-index --add frotz &&\n- if read_tree_u_must_succeed -m -u $treeH $treeM; then false; else :; fi'\n+ ! read_tree_u_must_succeed -m -u $treeH $treeM'\n \n test_expect_success \\\n '9 - conflicting addition.' \\\n@@ -163,7 +163,7 @@ test_expect_success \\\n echo frotz frotz >frotz &&\n git update-index --add frotz &&\n echo frotz >frotz &&\n- if read_tree_u_must_succeed -m -u $treeH $treeM; then false; else :; fi'\n+ ! read_tree_u_must_succeed -m -u $treeH $treeM'\n \n test_expect_success \\\n '10 - path removed.' \\\n@@ -186,7 +186,7 @@ test_expect_success \\\n echo rezrov >rezrov &&\n git update-index --add rezrov &&\n echo rezrov rezrov >rezrov &&\n- if read_tree_u_must_succeed -m -u $treeH $treeM; then false; else :; fi'\n+ ! read_tree_u_must_succeed -m -u $treeH $treeM'\n \n test_expect_success \\\n '12 - unmatching local changes being removed.' \\\n@@ -194,7 +194,7 @@ test_expect_success \\\n read_tree_u_must_succeed --reset -u $treeH &&\n echo rezrov rezrov >rezrov &&\n git update-index --add rezrov &&\n- if read_tree_u_must_succeed -m -u $treeH $treeM; then false; else :; fi'\n+ ! read_tree_u_must_succeed -m -u $treeH $treeM'\n \n test_expect_success \\\n '13 - unmatching local changes being removed.' \\\n@@ -203,7 +203,7 @@ test_expect_success \\\n echo rezrov rezrov >rezrov &&\n git update-index --add rezrov &&\n echo rezrov >rezrov &&\n- if read_tree_u_must_succeed -m -u $treeH $treeM; then false; else :; fi'\n+ ! read_tree_u_must_succeed -m -u $treeH $treeM'\n \n cat >expected <bozbar &&\n git update-index --add bozbar &&\n- if read_tree_u_must_succeed -m -u $treeH $treeM; then false; else :; fi'\n+ ! read_tree_u_must_succeed -m -u $treeH $treeM'\n \n test_expect_success \\\n '17 - conflicting local change.' \\\n@@ -260,7 +260,7 @@ test_expect_success \\\n echo bozbar bozbar >bozbar &&\n git update-index --add bozbar &&\n echo bozbar bozbar bozbar >bozbar &&\n- if read_tree_u_must_succeed -m -u $treeH $treeM; then false; else :; fi'\n+ ! read_tree_u_must_succeed -m -u $treeH $treeM'\n \n test_expect_success \\\n '18 - local change already having a good result.' \\\n@@ -316,7 +316,7 @@ test_expect_success \\\n echo bozbar >bozbar &&\n git update-index --add bozbar &&\n echo gnusto gnusto >bozbar &&\n- if read_tree_u_must_succeed -m -u $treeH $treeM; then false; else :; fi'\n+ ! read_tree_u_must_succeed -m -u $treeH $treeM'\n \n # Also make sure we did not break DF vs DF/DF case.\n test_expect_success \\\n\nbase-commit: d420dda0576340909c3faff364cfbd1485f70376\n-- \ngitgitgadget\n"},{"id":"464912","messageId":"pull.1362.v3.git.git.1665734502591.gitgitgadget@gmail.com","threadId":"58625","inReplyTo":"pull.1362.v2.git.git.1665733647421.gitgitgadget@gmail.com","subject":"[PATCH v3] [OUTREACHY] t1002: modernize outdated conditional","fromName":"nsengaw4c via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-10-14T08:01:42Z","receivedAt":"2022-10-14T08:01:51Z","isPatch":true,"sender":{"key":"name:nsengaw4c","avatar":null},"body":"From: Nsengiyumva Wilberforce \n\nTests in this script use an unusual and hard to reason about\nconditional construct\n\n if expression; then false; else :; fi\n\nChange them to use more idiomatic construct:\n\n ! expression\n\nCc: Christian Couder \nCc: Hariom Verma \nSigned-off-by: Nsengiyumva Wilberforce \n---\n [OUTREACHY]cleaning t1002-read-tree-m-u-2way.sh\n \n This is an update in t1002-read-tree-m-u-2way.sh. all the tests that use\n the unusual construct: if read_tree_u_must_succeed -m -u $treeH $treeM;\n then false; else :; fi have been updated to ! read_tree_u_must_succeed\n -m -u $treeH $treeM \"I am an outreachy applicant\" CC: Christian Couder\n christian.couder@gmail.com, Hariom verma hariom18599@gmail.com\n Signed-off-by: wilberforce nsengiyumvawilberforce@gmail.com\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1362%2Fnsengiyumva-wilberforce%2Ft1002_usual_construct_updated-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1362/nsengiyumva-wilberforce/t1002_usual_construct_updated-v3\nPull-Request: https://github.com/git/git/pull/1362\n\nRange-diff vs v2:\n\n 1: 8a9cd66d7d9 ! 1: d019ce50dc9 [OUTREACHY] t1002: modernize outdated conditional\n @@ Commit message\n \n Cc: Christian Couder \n Cc: Hariom Verma \n - Signed-off-by: Nsengiyumva Wilberfore \n + Signed-off-by: Nsengiyumva Wilberforce \n \n ## t/t1002-read-tree-m-u-2way.sh ##\n @@ t/t1002-read-tree-m-u-2way.sh: test_expect_success \\\n\n\n t/t1002-read-tree-m-u-2way.sh | 16 ++++++++--------\n 1 file changed, 8 insertions(+), 8 deletions(-)\n\ndiff --git a/t/t1002-read-tree-m-u-2way.sh b/t/t1002-read-tree-m-u-2way.sh\nindex bd5313caec9..cdc077ce12d 100755\n--- a/t/t1002-read-tree-m-u-2way.sh\n+++ b/t/t1002-read-tree-m-u-2way.sh\n@@ -154,7 +154,7 @@ test_expect_success \\\n read_tree_u_must_succeed --reset -u $treeH &&\n echo frotz frotz >frotz &&\n git update-index --add frotz &&\n- if read_tree_u_must_succeed -m -u $treeH $treeM; then false; else :; fi'\n+ ! read_tree_u_must_succeed -m -u $treeH $treeM'\n \n test_expect_success \\\n '9 - conflicting addition.' \\\n@@ -163,7 +163,7 @@ test_expect_success \\\n echo frotz frotz >frotz &&\n git update-index --add frotz &&\n echo frotz >frotz &&\n- if read_tree_u_must_succeed -m -u $treeH $treeM; then false; else :; fi'\n+ ! read_tree_u_must_succeed -m -u $treeH $treeM'\n \n test_expect_success \\\n '10 - path removed.' \\\n@@ -186,7 +186,7 @@ test_expect_success \\\n echo rezrov >rezrov &&\n git update-index --add rezrov &&\n echo rezrov rezrov >rezrov &&\n- if read_tree_u_must_succeed -m -u $treeH $treeM; then false; else :; fi'\n+ ! read_tree_u_must_succeed -m -u $treeH $treeM'\n \n test_expect_success \\\n '12 - unmatching local changes being removed.' \\\n@@ -194,7 +194,7 @@ test_expect_success \\\n read_tree_u_must_succeed --reset -u $treeH &&\n echo rezrov rezrov >rezrov &&\n git update-index --add rezrov &&\n- if read_tree_u_must_succeed -m -u $treeH $treeM; then false; else :; fi'\n+ ! read_tree_u_must_succeed -m -u $treeH $treeM'\n \n test_expect_success \\\n '13 - unmatching local changes being removed.' \\\n@@ -203,7 +203,7 @@ test_expect_success \\\n echo rezrov rezrov >rezrov &&\n git update-index --add rezrov &&\n echo rezrov >rezrov &&\n- if read_tree_u_must_succeed -m -u $treeH $treeM; then false; else :; fi'\n+ ! read_tree_u_must_succeed -m -u $treeH $treeM'\n \n cat >expected <bozbar &&\n git update-index --add bozbar &&\n- if read_tree_u_must_succeed -m -u $treeH $treeM; then false; else :; fi'\n+ ! read_tree_u_must_succeed -m -u $treeH $treeM'\n \n test_expect_success \\\n '17 - conflicting local change.' \\\n@@ -260,7 +260,7 @@ test_expect_success \\\n echo bozbar bozbar >bozbar &&\n git update-index --add bozbar &&\n echo bozbar bozbar bozbar >bozbar &&\n- if read_tree_u_must_succeed -m -u $treeH $treeM; then false; else :; fi'\n+ ! read_tree_u_must_succeed -m -u $treeH $treeM'\n \n test_expect_success \\\n '18 - local change already having a good result.' \\\n@@ -316,7 +316,7 @@ test_expect_success \\\n echo bozbar >bozbar &&\n git update-index --add bozbar &&\n echo gnusto gnusto >bozbar &&\n- if read_tree_u_must_succeed -m -u $treeH $treeM; then false; else :; fi'\n+ ! read_tree_u_must_succeed -m -u $treeH $treeM'\n \n # Also make sure we did not break DF vs DF/DF case.\n test_expect_success \\\n\nbase-commit: d420dda0576340909c3faff364cfbd1485f70376\n-- \ngitgitgadget\n"},{"id":"464963","messageId":"xmqqv8om9yaz.fsf@gitster.g","threadId":"58625","inReplyTo":"pull.1362.v3.git.git.1665734502591.gitgitgadget@gmail.com","subject":"Re: [PATCH v3] [OUTREACHY] t1002: modernize outdated conditional","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-10-14T16:15:16Z","receivedAt":"2022-10-14T16:15:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"nsengaw4c via GitGitGadget\" writes:\n\n> From: Nsengiyumva Wilberforce \n>\n> Tests in this script use an unusual and hard to reason about\n> conditional construct\n>\n> if expression; then false; else :; fi\n>\n> Change them to use more idiomatic construct:\n>\n> ! expression\n>\n> Cc: Christian Couder \n> Cc: Hariom Verma \n> Signed-off-by: Nsengiyumva Wilberforce \n\nWhat are these C: lines for? I do not think the message I am\nresponding to is Cc'ed to them. There may be a special incantation\nto tell GitGitGadget to Cc to certain folks, but adding Cc: to the\nlog message trailer like this does not seem to be it---at least it\nappears that it did not work that way.\n\n> ...\n> - if read_tree_u_must_succeed -m -u $treeH $treeM; then false; else :; fi'\n> + ! read_tree_u_must_succeed -m -u $treeH $treeM'\n\nLooks good. For the purpose of microproject, I think this is a good\nplace to stop, as it does not make anything worse and make the code\nprettier.\n\nTo those more experienced contributors who are watching from\nsidelines, and especially to our mentors, it may be worth taking a\nlook at the implementation of the helper shell function used here,\nand think if it makes sense to expect a failure with a simple \"!\"\nprefix (or with the original long hand if/then/else/fi that has\nexactly the same issue).\n\nread_tree_u_must_succeed () {\n\tgit ls-files -s >pre-dry-run &&\n\tgit diff-files -p >pre-dry-run-wt &&\n\tgit read-tree -n \"$@\" &&\n\tgit ls-files -s >post-dry-run &&\n\tgit diff-files -p >post-dry-run-wt &&\n\ttest_cmp pre-dry-run post-dry-run &&\n\ttest_cmp pre-dry-run-wt post-dry-run-wt &&\n\tgit read-tree \"$@\"\n}\n\nWhat if read-tree segfaults? This entire function will fail and the\ntest that runs read_tree_u_must_succeed and negates its result would\nbe a poor fit here.\n\nThanks.\n"},{"id":"464964","messageId":"f064ce46-8ed0-a9c1-8df5-5c258677d95f@github.com","threadId":"58625","inReplyTo":"xmqqv8om9yaz.fsf@gitster.g","subject":"Re: [PATCH v3] [OUTREACHY] t1002: modernize outdated conditional","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-10-14T16:21:05Z","receivedAt":"2022-10-14T16:21:15Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 10/14/2022 12:15 PM, Junio C Hamano wrote:\n> \"nsengaw4c via GitGitGadget\" writes:\n> \n>> From: Nsengiyumva Wilberforce \n>>\n>> Tests in this script use an unusual and hard to reason about\n>> conditional construct\n>>\n>> if expression; then false; else :; fi\n>>\n>> Change them to use more idiomatic construct:\n>>\n>> ! expression\n>>\n>> Cc: Christian Couder \n>> Cc: Hariom Verma \n>> Signed-off-by: Nsengiyumva Wilberforce \n> \n> What are these C: lines for? I do not think the message I am\n> responding to is Cc'ed to them. There may be a special incantation\n> to tell GitGitGadget to Cc to certain folks, but adding Cc: to the\n> log message trailer like this does not seem to be it---at least it\n> appears that it did not work that way.\n\nGitGitGadget will read the \"cc:\" lines from the end of the pull request\ndescription, not the commit messages. I'm pretty sure they will be\nignored if there are other lines after them.\n\nThanks,\n-Stolee\n"},{"id":"464966","messageId":"CAPig+cT=bJ7BP9CDh5-oYYF376vVxsh7E0UAE_QN0wfAgR3AAg@mail.gmail.com","threadId":"58625","inReplyTo":"f064ce46-8ed0-a9c1-8df5-5c258677d95f@github.com","subject":"Re: [PATCH v3] [OUTREACHY] t1002: modernize outdated conditional","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2022-10-14T16:58:24Z","receivedAt":"2022-10-14T16:58:51Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Oct 14, 2022 at 12:35 PM Derrick Stolee\n wrote:\n> On 10/14/2022 12:15 PM, Junio C Hamano wrote:\n> > \"nsengaw4c via GitGitGadget\" writes:\n> >> Cc: Christian Couder \n> >> Cc: Hariom Verma \n> >\n> > What are these C: lines for? I do not think the message I am\n> > responding to is Cc'ed to them. There may be a special incantation\n> > to tell GitGitGadget to Cc to certain folks, but adding Cc: to the\n> > log message trailer like this does not seem to be it---at least it\n> > appears that it did not work that way.\n>\n> GitGitGadget will read the \"cc:\" lines from the end of the pull request\n> description, not the commit messages. I'm pretty sure they will be\n> ignored if there are other lines after them.\n\nFor Wilberforce's edification for future submissions, presumably the\nreason that the CC: in the pull-request's description didn't work is\nbecause the CC: line wasn't the last line in the description? Does\nthere need to be a blank line before the CC: line? Is it okay to list\nmultiple people on the same CC: line as done in this case, or is that\nalso a problem?\n"},{"id":"464969","messageId":"cc0a6adb-d894-77b3-2a65-9042237c07b5@github.com","threadId":"58625","inReplyTo":"CAPig+cT=bJ7BP9CDh5-oYYF376vVxsh7E0UAE_QN0wfAgR3AAg@mail.gmail.com","subject":"Re: [PATCH v3] [OUTREACHY] t1002: modernize outdated conditional","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-10-14T17:54:39Z","receivedAt":"2022-10-14T17:54:46Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 10/14/2022 12:58 PM, Eric Sunshine wrote:\n> On Fri, Oct 14, 2022 at 12:35 PM Derrick Stolee\n> wrote:\n>> On 10/14/2022 12:15 PM, Junio C Hamano wrote:\n>>> \"nsengaw4c via GitGitGadget\" writes:\n>>>> Cc: Christian Couder \n>>>> Cc: Hariom Verma \n>>>\n>>> What are these C: lines for? I do not think the message I am\n>>> responding to is Cc'ed to them. There may be a special incantation\n>>> to tell GitGitGadget to Cc to certain folks, but adding Cc: to the\n>>> log message trailer like this does not seem to be it---at least it\n>>> appears that it did not work that way.\n>>\n>> GitGitGadget will read the \"cc:\" lines from the end of the pull request\n>> description, not the commit messages. I'm pretty sure they will be\n>> ignored if there are other lines after them.\n> \n> For Wilberforce's edification for future submissions, presumably the\n> reason that the CC: in the pull-request's description didn't work is\n> because the CC: line wasn't the last line in the description? Does\n> there need to be a blank line before the CC: line? Is it okay to list\n> multiple people on the same CC: line as done in this case, or is that\n> also a problem?\n\nLooking at the PR (https://github.com/git/git/pull/1362) it seems\nthere was no \"cc:\" lines in the PR description (until GitGitGadget\nadded them due to our replies).\n\nNsengiyumva: you'll want to be careful to edit your pull request\ndescription on GitHub before running the \"/submit\" chatop. Your\ncurrent description has a paste of your commit message followed by\nthe contributing template. The pull request description becomes\nyour cover letter (in the case of multiple patches) or a commentary\nsection after the commit message (in this case of a single patch).\n\nThe description is a good place to say things like \"I started\nworking on this because of a mailing list thread...\" or \"I'm not\nsure if I've tested everything enough\".\n\nThe \"cc:\" lines should _not_ be in the commit message at all, which\nis what's visible in the patch.\n\nThanks,\n-Stolee\n"},{"id":"464971","messageId":"pull.1362.v4.git.git.1665772130030.gitgitgadget@gmail.com","threadId":"58625","inReplyTo":"pull.1362.v3.git.git.1665734502591.gitgitgadget@gmail.com","subject":"[PATCH v4] [OUTREACHY] t1002: modernize outdated conditional","fromName":"nsengaw4c via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-10-14T18:28:49Z","receivedAt":"2022-10-14T18:28:57Z","isPatch":true,"sender":{"key":"name:nsengaw4c","avatar":null},"body":"From: Nsengiyumva Wilberforce \n\nTests in this script use an unusual and hard to reason about\nconditional construct\n\n if expression; then false; else :; fi\n\nChange them to use more idiomatic construct:\n\n ! expression\n\nSigned-off-by: Nsengiyumva Wilberforce \n---\n [OUTREACHY]cleaning t1002-read-tree-m-u-2way.sh\n \n This is an update in t1002-read-tree-m-u-2way.sh. all the tests that use\n the unusual construct: if read_tree_u_must_succeed -m -u $treeH $treeM;\n then false; else :; fi have been updated to ! read_tree_u_must_succeed\n -m -u $treeH $treeM \"I am an outreachy applicant\" CC: Christian Couder\n christian.couder@gmail.com, Hariom verma hariom18599@gmail.com\n Signed-off-by: wilberforce nsengiyumvawilberforce@gmail.com\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1362%2Fnsengiyumva-wilberforce%2Ft1002_usual_construct_updated-v4\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1362/nsengiyumva-wilberforce/t1002_usual_construct_updated-v4\nPull-Request: https://github.com/git/git/pull/1362\n\nRange-diff vs v3:\n\n 1: d019ce50dc9 ! 1: c0109d947d4 [OUTREACHY] t1002: modernize outdated conditional\n @@ Commit message\n \n ! expression\n \n - Cc: Christian Couder \n - Cc: Hariom Verma \n Signed-off-by: Nsengiyumva Wilberforce \n \n ## t/t1002-read-tree-m-u-2way.sh ##\n\n\n t/t1002-read-tree-m-u-2way.sh | 16 ++++++++--------\n 1 file changed, 8 insertions(+), 8 deletions(-)\n\ndiff --git a/t/t1002-read-tree-m-u-2way.sh b/t/t1002-read-tree-m-u-2way.sh\nindex bd5313caec9..cdc077ce12d 100755\n--- a/t/t1002-read-tree-m-u-2way.sh\n+++ b/t/t1002-read-tree-m-u-2way.sh\n@@ -154,7 +154,7 @@ test_expect_success \\\n read_tree_u_must_succeed --reset -u $treeH &&\n echo frotz frotz >frotz &&\n git update-index --add frotz &&\n- if read_tree_u_must_succeed -m -u $treeH $treeM; then false; else :; fi'\n+ ! read_tree_u_must_succeed -m -u $treeH $treeM'\n \n test_expect_success \\\n '9 - conflicting addition.' \\\n@@ -163,7 +163,7 @@ test_expect_success \\\n echo frotz frotz >frotz &&\n git update-index --add frotz &&\n echo frotz >frotz &&\n- if read_tree_u_must_succeed -m -u $treeH $treeM; then false; else :; fi'\n+ ! read_tree_u_must_succeed -m -u $treeH $treeM'\n \n test_expect_success \\\n '10 - path removed.' \\\n@@ -186,7 +186,7 @@ test_expect_success \\\n echo rezrov >rezrov &&\n git update-index --add rezrov &&\n echo rezrov rezrov >rezrov &&\n- if read_tree_u_must_succeed -m -u $treeH $treeM; then false; else :; fi'\n+ ! read_tree_u_must_succeed -m -u $treeH $treeM'\n \n test_expect_success \\\n '12 - unmatching local changes being removed.' \\\n@@ -194,7 +194,7 @@ test_expect_success \\\n read_tree_u_must_succeed --reset -u $treeH &&\n echo rezrov rezrov >rezrov &&\n git update-index --add rezrov &&\n- if read_tree_u_must_succeed -m -u $treeH $treeM; then false; else :; fi'\n+ ! read_tree_u_must_succeed -m -u $treeH $treeM'\n \n test_expect_success \\\n '13 - unmatching local changes being removed.' \\\n@@ -203,7 +203,7 @@ test_expect_success \\\n echo rezrov rezrov >rezrov &&\n git update-index --add rezrov &&\n echo rezrov >rezrov &&\n- if read_tree_u_must_succeed -m -u $treeH $treeM; then false; else :; fi'\n+ ! read_tree_u_must_succeed -m -u $treeH $treeM'\n \n cat >expected <bozbar &&\n git update-index --add bozbar &&\n- if read_tree_u_must_succeed -m -u $treeH $treeM; then false; else :; fi'\n+ ! read_tree_u_must_succeed -m -u $treeH $treeM'\n \n test_expect_success \\\n '17 - conflicting local change.' \\\n@@ -260,7 +260,7 @@ test_expect_success \\\n echo bozbar bozbar >bozbar &&\n git update-index --add bozbar &&\n echo bozbar bozbar bozbar >bozbar &&\n- if read_tree_u_must_succeed -m -u $treeH $treeM; then false; else :; fi'\n+ ! read_tree_u_must_succeed -m -u $treeH $treeM'\n \n test_expect_success \\\n '18 - local change already having a good result.' \\\n@@ -316,7 +316,7 @@ test_expect_success \\\n echo bozbar >bozbar &&\n git update-index --add bozbar &&\n echo gnusto gnusto >bozbar &&\n- if read_tree_u_must_succeed -m -u $treeH $treeM; then false; else :; fi'\n+ ! read_tree_u_must_succeed -m -u $treeH $treeM'\n \n # Also make sure we did not break DF vs DF/DF case.\n test_expect_success \\\n\nbase-commit: d420dda0576340909c3faff364cfbd1485f70376\n-- \ngitgitgadget\n"},{"id":"464975","messageId":"xmqqlepi8c7o.fsf@gitster.g","threadId":"58625","inReplyTo":"cc0a6adb-d894-77b3-2a65-9042237c07b5@github.com","subject":"Re: [PATCH v3] [OUTREACHY] t1002: modernize outdated conditional","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-10-14T18:57:47Z","receivedAt":"2022-10-14T18:57:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Derrick Stolee writes:\n\n> Nsengiyumva: you'll want to be careful to edit your pull request\n> description on GitHub before running the \"/submit\" chatop. Your\n> current description has a paste of your commit message followed by\n> the contributing template. The pull request description becomes\n> your cover letter (in the case of multiple patches) or a commentary\n> section after the commit message (in this case of a single patch).\n>\n> The description is a good place to say things like \"I started\n> working on this because of a mailing list thread...\" or \"I'm not\n> sure if I've tested everything enough\".\n\nGood advice.\n\n> The \"cc:\" lines should _not_ be in the commit message at all, which\n> is what's visible in the patch.\n\nI agree that it would probably be better without CC: in the trailer\nin this case. Some projects seem to use the CC: in the trailer to\nsignal that these people have been notified, without implying\nanything about their reaction (i.e. when the author cannot use\nreviewed-by or acked-by). We are not that large a community, so I\npersonally do not see a need to use such a trailer around here. But\nthat is only a local convention in this project.\n\nThanks.\n\n"},{"id":"464976","messageId":"xmqqh7068bta.fsf@gitster.g","threadId":"58625","inReplyTo":"CAPig+cT=bJ7BP9CDh5-oYYF376vVxsh7E0UAE_QN0wfAgR3AAg@mail.gmail.com","subject":"Re: [PATCH v3] [OUTREACHY] t1002: modernize outdated conditional","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-10-14T19:06:25Z","receivedAt":"2022-10-14T19:06:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n> On Fri, Oct 14, 2022 at 12:35 PM Derrick Stolee\n> wrote:\n>> On 10/14/2022 12:15 PM, Junio C Hamano wrote:\n>> > \"nsengaw4c via GitGitGadget\" writes:\n>> >> Cc: Christian Couder \n>> >> Cc: Hariom Verma \n>> >\n>> > What are these C: lines for? I do not think the message I am\n>> > responding to is Cc'ed to them. There may be a special incantation\n>> > to tell GitGitGadget to Cc to certain folks, but adding Cc: to the\n>> > log message trailer like this does not seem to be it---at least it\n>> > appears that it did not work that way.\n>>\n>> GitGitGadget will read the \"cc:\" lines from the end of the pull request\n>> description, not the commit messages. I'm pretty sure they will be\n>> ignored if there are other lines after them.\n>\n> For Wilberforce's edification for future submissions, presumably the\n> reason that the CC: in the pull-request's description didn't work is\n> because the CC: line wasn't the last line in the description? Does\n> there need to be a blank line before the CC: line? Is it okay to list\n> multiple people on the same CC: line as done in this case, or is that\n> also a problem?\n\nAh, now I can see why the round v4 is CC'ed to you and Derrick on\nthe list. The pull-request text (visible in GitHub UI in the top\nmost box of https://github.com/git/git/pull/1362) ends with two\nlines of cc: that list you two. The one named Christian and Hariom\nwere not at the end and was ignored by GGG, it seems.\n\n"},{"id":"464977","messageId":"CAPig+cR2R3EY=53ELaFY3wqy7danQmHNm0Qeqqh9nW7n8XHNHg@mail.gmail.com","threadId":"58625","inReplyTo":"xmqqh7068bta.fsf@gitster.g","subject":"Re: [PATCH v3] [OUTREACHY] t1002: modernize outdated conditional","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2022-10-14T19:14:24Z","receivedAt":"2022-10-14T19:14:45Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Oct 14, 2022 at 3:06 PM Junio C Hamano wrote:\n> Eric Sunshine writes:\n> > On Fri, Oct 14, 2022 at 12:35 PM Derrick Stolee\n> > wrote:\n> >> GitGitGadget will read the \"cc:\" lines from the end of the pull request\n> >> description, not the commit messages. I'm pretty sure they will be\n> >> ignored if there are other lines after them.\n> >\n> > For Wilberforce's edification for future submissions, presumably the\n> > reason that the CC: in the pull-request's description didn't work is\n> > because the CC: line wasn't the last line in the description? Does\n> > there need to be a blank line before the CC: line? Is it okay to list\n> > multiple people on the same CC: line as done in this case, or is that\n> > also a problem?\n>\n> Ah, now I can see why the round v4 is CC'ed to you and Derrick on\n> the list. The pull-request text (visible in GitHub UI in the top\n> most box of https://github.com/git/git/pull/1362) ends with two\n> lines of cc: that list you two. The one named Christian and Hariom\n> were not at the end and was ignored by GGG, it seems.\n\nYes, the CC: line mentioning Christian and Hariom was not at the end\nof the description, which is likely why GitGitGadget didn't pick it\nup. (Presumably Stolee overlooked that line when responding to my\nquestion.) However, clarification about whether or not there needs to\nbe a blank line before the CC: line would be nice (I presume the blank\nline is needed), but also whether or not GitGitGadget correctly deals\nwith multiple people mentioned on the same CC: line or if they each\nneed to occupy a single CC: line.\n"},{"id":"464984","messageId":"692dbb0d-a3f9-7e12-c868-fffc8df4678b@iee.email","threadId":"58625","inReplyTo":"xmqqh7068bta.fsf@gitster.g","subject":"Re: [PATCH v3] [OUTREACHY] t1002: modernize outdated conditional","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2022-10-14T20:19:54Z","receivedAt":"2022-10-14T20:20:07Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"On 14/10/2022 20:06, Junio C Hamano wrote:\n> Eric Sunshine writes:\n>\n>> On Fri, Oct 14, 2022 at 12:35 PM Derrick Stolee\n>> wrote:\n>>> On 10/14/2022 12:15 PM, Junio C Hamano wrote:\n>>>> \"nsengaw4c via GitGitGadget\" writes:\n>>>>> Cc: Christian Couder \n>>>>> Cc: Hariom Verma \n>>>> What are these C: lines for? I do not think the message I am\n>>>> responding to is Cc'ed to them. There may be a special incantation\n>>>> to tell GitGitGadget to Cc to certain folks, but adding Cc: to the\n>>>> log message trailer like this does not seem to be it---at least it\n>>>> appears that it did not work that way.\n>>> GitGitGadget will read the \"cc:\" lines from the end of the pull request\n>>> description, not the commit messages. I'm pretty sure they will be\n>>> ignored if there are other lines after them.\n>> For Wilberforce's edification for future submissions, presumably the\n>> reason that the CC: in the pull-request's description didn't work is\n>> because the CC: line wasn't the last line in the description? Does\n>> there need to be a blank line before the CC: line? Is it okay to list\n>> multiple people on the same CC: line as done in this case, or is that\n>> also a problem?\n> Ah, now I can see why the round v4 is CC'ed to you and Derrick on\n> the list. The pull-request text (visible in GitHub UI in the top\n> most box of https://github.com/git/git/pull/1362) ends with two\n> lines of cc: that list you two. The one named Christian and Hariom\n> were not at the end and was ignored by GGG, it seems.\n>\nI just want to throw in that because GitHub takes the PR & comitts\nverbatim, but Git itself works via email, you can add description\nportions to commits, and I believe the PR part, by add a line containing\njust three dashes `---` followed by the additional descriptive note text\nwhich won't be used when `am` (apply mailbox) is used.\n\nI've certainly used that technique when sending patches. See the \"Bonus\nChapter: One-Patch Changes\" in MyFirstContribution.txt\n--\nPhilip\n"},{"id":"464988","messageId":"xmqq4jw687f0.fsf@gitster.g","threadId":"58625","inReplyTo":"CAPig+cR2R3EY=53ELaFY3wqy7danQmHNm0Qeqqh9nW7n8XHNHg@mail.gmail.com","subject":"Re: [PATCH v3] [OUTREACHY] t1002: modernize outdated conditional","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-10-14T20:41:23Z","receivedAt":"2022-10-14T20:41:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n> ... However, clarification about whether or not there needs to\n> be a blank line before the CC: line would be nice (I presume the blank\n> line is needed), but also whether or not GitGitGadget correctly deals\n> with multiple people mentioned on the same CC: line or if they each\n> need to occupy a single CC: line.\n\nIndeed it is very good to have such a documentation that tells us\nall these things. Is the \"Welcome to GGG\" comment it adds to first\ntime users a good place to have this kind of information (I am\nguessing not, as more advanced features may become needed after you\nused the tool several times)?\n"},{"id":"464989","messageId":"xmqqzgdy6srz.fsf@gitster.g","threadId":"58625","inReplyTo":"692dbb0d-a3f9-7e12-c868-fffc8df4678b@iee.email","subject":"Re: [PATCH v3] [OUTREACHY] t1002: modernize outdated conditional","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-10-14T20:42:56Z","receivedAt":"2022-10-14T20:43:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Philip Oakley writes:\n\n> I just want to throw in that because GitHub takes the PR & comitts\n> verbatim, but Git itself works via email, you can add description\n> portions to commits, and I believe the PR part, by add a line containing\n> just three dashes `---` followed by the additional descriptive note text\n> which won't be used when `am` (apply mailbox) is used.\n\nYup, if you are absolutely sure you won't interact with others in\nany way other than e-mailed patches, it is a great trick to use.\n"}]}