All of lore.kernel.org
 help / color / mirror / Atom feed
From: Shourya Shukla <shouryashukla.oo@gmail.com>
To: shouryashukla.oo@gmail.com
Cc: christian.couder@gmail.com, git@vger.kernel.org,
	gitster@pobox.com, Johannes.Schindelin@gmx.de,
	kaartic.sivaraam@gmail.com, liu.denton@gmail.com
Subject: [GSoC][PATCH v2 0/4] t7401: modernize, cleanup and more
Date: Thu, 13 Aug 2020 00:57:33 +0530	[thread overview]
Message-ID: <20200812192737.13971-1-shouryashukla.oo@gmail.com> (raw)
In-Reply-To: <20200805174921.16000-1-shouryashukla.oo@gmail.com>

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@gmail.com/

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@tuxfamily.org>
         Mentored-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
    +    Helped-by: Denton Liu <liu.denton@gmail.com>
    +    Helped-by: Taylor Blau <me@ttaylorr.com>
         Signed-off-by: Shourya Shukla <shouryashukla.oo@gmail.com>

      ## 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@gmail.com>

      ## 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@tuxfamily.org>
         Mentored-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
    +    Helped-by: Taylor Blau <me@ttaylorr.com>
         Signed-off-by: Shourya Shukla <shouryashukla.oo@gmail.com>

      ## 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


  parent reply	other threads:[~2020-08-12 19:27 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-05 17:49 [GSoC][RESEND][PATCH 0/4] t7401: modernize, cleanup and warn Shourya Shukla
2020-08-05 17:49 ` [PATCH 1/4] t7401: modernize style Shourya Shukla
2020-08-05 19:37   ` Denton Liu
2020-08-05 20:49     ` Taylor Blau
2020-08-06  8:45       ` Shourya Shukla
2020-08-05 17:49 ` [PATCH 2/4] t7401: change test_i18ncmp syntax for clarity Shourya Shukla
2020-08-05 20:58   ` Taylor Blau
2020-08-05 21:23   ` Junio C Hamano
2020-08-05 17:49 ` [PATCH 3/4] t7401: ensure uniformity in the '--for-status' test Shourya Shukla
2020-08-05 21:01   ` Taylor Blau
2020-08-05 22:25     ` Junio C Hamano
2020-08-05 22:26       ` Taylor Blau
2020-08-05 21:30   ` Junio C Hamano
2020-08-06  8:50     ` Shourya Shukla
2020-08-06 17:18       ` Junio C Hamano
2020-08-05 17:49 ` [PATCH 4/4] t7401: add a WARNING and a NEEDSWORK Shourya Shukla
2020-08-05 21:04   ` Taylor Blau
2020-08-05 21:36   ` Junio C Hamano
2020-08-06 11:27     ` Shourya Shukla
2020-08-06 21:11       ` Junio C Hamano
2020-08-07 14:55         ` Christian Couder
2020-08-12 19:27 ` Shourya Shukla [this message]
2020-08-12 19:27   ` [PATCH v2 1/4] t7401: modernize style Shourya Shukla
2020-08-13  8:06     ` Kaartic Sivaraam
2020-08-13 16:46       ` Junio C Hamano
2020-08-14 14:41         ` Shourya Shukla
2020-08-14 17:06           ` Junio C Hamano
2020-08-12 19:27   ` [PATCH v2 2/4] t7401: change test_i18ncmp syntax for clarity Shourya Shukla
2020-08-12 20:57     ` Junio C Hamano
2020-08-12 21:02       ` Junio C Hamano
2020-08-14 14:57         ` Shourya Shukla
2020-08-12 19:27   ` [PATCH v2 3/4] t7401: change indentation for enhanced readability Shourya Shukla
2020-08-12 19:27   ` [PATCH v2 4/4] t7401: add a NEEDSWORK Shourya Shukla

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200812192737.13971-1-shouryashukla.oo@gmail.com \
    --to=shouryashukla.oo@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=kaartic.sivaraam@gmail.com \
    --cc=liu.denton@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.