On Sun, Oct 6, 2024 at 2:31 AM Usman Akinyemi
[off-list ref] wrote:

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

quoted

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).

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().

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 ?

I'm not sure which three patches you mean. Does the third patch, the
"new one on the same branch", fix problems from the earlier two
patches? Or does your third patch implement the suggestion regarding
test_line_count()?

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 ?

A patch series consists of one or more patches in sequence. Patches
within a series don't conflict with one another; later patches build
upon earlier patches. You create a series of commits on a single Git
branch. When you submit that branch as a pull-request via
GitGitGadget, it turns the commits on that branch into a series of
patch emails to the Git mailing list, one per commit.

Before submitting the patches via GitGitGadget, you polish them
locally to repair any problems before submitting them for review.
Rather than making subsequent commits fix problems with earlier
commits, you instead should fix the bad commits by using "git rebase
--interactive ..." to either "fixup", "squash", or otherwise edit the
bad commits. Once you are happy with the commits, submit them. This
way, reviewers only see your polished result, not the series of steps
you made to arrive at the polished results.

You do the same thing after reviewers point out problems or ask for
changes. Edit and re-polish the existing commits to address reviewer
comments rather than merely making new commits on top of the existing
commits, and then resubmit once all the fixes have been applied and
polished.

When I suggested squashing your two original commits it was for the
above reason. In your original submission, patch [1/2] had some
problems which you fixed in patch [2/2], but reviewers don't need or
want to see that; they just want to see the polished end-result, which
you can obtain by squashing the two patches together. (However, in
this case, as I pointed out in my review, you don't even need to use
`tr`; just use `test 1 = $count` instead of `test 1 = "$count"`.)

If you wanted to do the extra step of also updating the tests to use
test_line_count(), then that would be a separate patch, still on the
same branch, built on top of your "fix Git upstream of pipe" patch.
Thus, it would become a two-patch series: patch [1/2] fixing Git
upstream of a pipe, and [2/2] employing test_line_count().

Also, to be clear, "Description" is the body of the commit message if
I use the gitgitgadget while the "commit message" is the header ?

The commit message is separate from the patch-series commentary. The
commit message of each patch explains what that patch changes or does.

Once you have polished your commit(s), force-push them to the
GitGitGadget pull-request you already created. Then edit the very
first (topmost) comment in the pull-request to explain what the patch
series is about and what you changed since the previous version. That
comment becomes the "commentary" portion of the patch series.