Greetings, This is the v2 of the previously posted patch series titled 't7401: modernize, cleanup and warn': https://lore.kernel.org/git/20200805174921.16000-1-shouryashukla.oo@xxxxxxxxx/ After suggestions from Denton, Taylor and Junio I made some changes: -> In 2939804509 (t7401: modernize style, 2020-07-23), after suggestions from Denton and Taylor, I redirected the output of the two 'rev-parse' commands to a file and then read from it instead of using a pipe '|' operator. -> Drop the commit ab28283b67 (t7401: ensure uniformity in the '--for-status' test, 2020-07-10) and instead combine it with 00c6289d5e (t7401: change test_i18ncmp syntax for clarity, 2020-07-10). This was suggested by Junio and Taylor. -> Introduce the commit f0b87ddaf6 (t7401: change indentation for enhanced readability, 2020-08-11) which improves the indentation of the tests in t7401. This was suggested by Junio and Taylor. -> Remove the WARNING from the commit 0bdb0bd72c (t7401: add a WARNING and a NEEDSWORK, 2020-07-23) and instead limit it to a more improved NEEDSWORK thus leading to the commit a743c28d71 (t7401: add a NEEDSWORK, 2020-07-23). This was suggested by Taylor. Feedback and reviews are appreciated. I am tagging along a range-diff between the v1 and v2 for ease of review. Regards, Shourya Shukla ----- range-diff: 1: 31ae4038f1 ! 1: 2939804509 t7401: modernize style @@ Commit message t7401: modernize style The tests in 't7401-submodule-summary.sh' were written a long time ago - and have some violations with respect to our CodingGuidelines such as - incorrect spacing in usages of the redirection operator and absence - of line feed between statements in case of the '|' (pipe) operator. - Update it to match the CodingGuidelines. + and has a violation with respect to our CodingGuidelines which is, + incorrect spacing in usages of the redirection operator. + Using a Git command in the upstream of a pipe might result in us + losing its exit code. So, convert such usages so that they write to + a file and read from them. Mentored-by: Christian Couder <chriscool@xxxxxxxxxxxxx> Mentored-by: Kaartic Sivaraam <kaartic.sivaraam@xxxxxxxxx> + Helped-by: Denton Liu <liu.denton@xxxxxxxxx> + Helped-by: Taylor Blau <me@xxxxxxxxxxxx> Signed-off-by: Shourya Shukla <shouryashukla.oo@xxxxxxxxx> ## t/t7401-submodule-summary.sh ## @@ t/t7401-submodule-summary.sh: add_file () { git commit -m "Add $name" git commit -m "Add $name" done >/dev/null - git rev-parse --verify HEAD | cut -c1-7 -+ git rev-parse --verify HEAD | -+ cut -c1-7 ++ git rev-parse --verify HEAD >out && ++ cut -c1-7 out cd "$owd" } commit_file () { @@ t/t7401-submodule-summary.sh: commit_file sm1 && cd sm1 && git reset --hard HEAD~2 >/dev/null && - git rev-parse --verify HEAD | cut -c1-7 -+ git rev-parse --verify HEAD | -+ cut -c1-7 ++ git rev-parse --verify HEAD >out && ++ cut -c1-7 out ) test_expect_success 'modified submodule(backward)' " 2: a3160d1ecc ! 2: 00c6289d5e t7401: change test_i18ncmp syntax for clarity @@ t/t7401-submodule-summary.sh: test_expect_success 'nonexistent commit' " " commit_file +@@ t/t7401-submodule-summary.sh: EOF + + test_expect_success '--for-status' " + git submodule summary --for-status HEAD^ >actual && +- test_i18ncmp actual - <<EOF ++ test_i18ncmp - actual <<-EOF + * sm1 $head6...0000000: + + * sm2 0000000...$head7 (2): 3: ab28283b67 < -: ---------- t7401: ensure uniformity in the '--for-status' test -: ---------- > 3: f0b87ddaf6 t7401: change indentation for enhanced readability 4: 0bdb0bd72c ! 4: a743c28d71 t7401: add a WARNING and a NEEDSWORK @@ Metadata Author: Shourya Shukla <shouryashukla.oo@xxxxxxxxx> ## Commit message ## - t7401: add a WARNING and a NEEDSWORK + t7401: add a NEEDSWORK - Add a WARNING regarding the usage of 'git add' instead of 'git - submodule add' to add submodules to the superproject. Also add a - NEEDSWORK regarding the outdated syntax and working of the test, which - may need to be improved to obtain better and desired results. + Add a NEEDSWORK regarding the outdated syntax and working of the test, + which may need to be improved to obtain better and desired results. While at it, change the word 'test' to 'test script' in the test description to avoid ambiguity. Mentored-by: Christian Couder <chriscool@xxxxxxxxxxxxx> Mentored-by: Kaartic Sivaraam <kaartic.sivaraam@xxxxxxxxx> + Helped-by: Taylor Blau <me@xxxxxxxxxxxx> Signed-off-by: Shourya Shukla <shouryashukla.oo@xxxxxxxxx> ## t/t7401-submodule-summary.sh ## @@ t/t7401-submodule-summary.sh -This test tries to verify the sanity of summary subcommand of git submodule. +This test script tries to verify the sanity of summary subcommand of git submodule. ' -+# WARNING: This test script uses 'git add' instead of 'git submodule add' to add -+# submodules to the superproject. Some submodule subcommands such as init and -+# deinit might not work as expected in this script. -+ -+# NEEDSWORK: This test script is old fashioned and may need a big cleanup. ++# NEEDSWORK: This test script is old fashioned and may need a big cleanup since ++# there are lots of commands taking place outside of 'test_expect_success' ++# block, which is no longer in good-style. . ./test-lib.sh ----- Shourya Shukla (4): t7401: modernize style t7401: change test_i18ncmp syntax for clarity t7401: change indentation for enhanced readability t7401: add a NEEDSWORK t/t7401-submodule-summary.sh | 151 ++++++++++++++++++----------------- 1 file changed, 78 insertions(+), 73 deletions(-) -- 2.28.0
