On Sun, Oct 6, 2024 at 5:48 AM Eric Sunshine [off-list ref] wrote:

On Sun, Oct 6, 2024 at 1:33 AM Usman Akinyemi via GitGitGadget
[off-list ref] wrote:

quoted

Changes since v1:
- Added "tr -d '[:space:]'" to handle whitespace on macOS

Signed-off-by: Usman Akinyemi <redacted>
---

Thanks for the submission. A few comments...

This second patch fixes problems with the first patch, but since this
is an entirely new submission, you should instead "squash" these two
patches together and then force-push them to the same branch that you
used when submitting them via GitGitGadget, and re-submit them as a
single patch. When you squash them, keep the commit message from the
first patch.

Reviewers do appreciate that you explained what changed since the
previous version, but we'd like to see that information as commentary
in the patch cover letter, not as the commit message of the patch
itself. In GitGitGadget, the way you would do so is to write this as
the "Description" of the pull-request (possibly replacing or amending
the previous description).

Some more observations below...

quoted

diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh

@@ -401,7 +401,7 @@ test_expect_success 'multi-squash only fires up editor once' '
        git show >output &&
-       count=$(grep ONCE output | wc -l) &&
+       count=$(grep ONCE output | wc -l | tr -d '[:space:]') &&
        test 1 = "$count"
 '

The reason this was failing for you was because you quoted $count. Had
you instead written:

    test 1 = $count

when it would have worked as expected. In other words, you don't need `tr`.

These days, instead of manually using `wc -l` and `test`, we would
instead write:

    grep ONCE output >actual &&
    test_line_count 1 actual

However, that sort of change is independent of the purpose of this
patch, so you probably should not make such a change in this patch. If
you're up to it, you could instead turn this into a two-patch series
in which patch [1/2] fixes the "Git upstream of a pipe" problem, and
then patch [2/2] converts these cases to use test_line_count().

Hi  Eric Sunshine,
thanks for the review. I really appreciate it. I have a couple of
doubts to clear.

My next patch should be the squash of my three patches which include
my first two patches and the new one on the same branch ?
Two patch series means two different commits on different patches ?
But, since they both depend on each other would not they lead to merge
conflict ?
Also, to be clear, "Description" is the body of the commit message if
I use the gitgitgadget while the "commit message" is the header ?

Thank you.