Re: [PATCH v2] SubmittingPatches: avoid add-failing-test-then-fix anti-pattern

[Junio C Hamano <gitster@pobox.com>]

Jon Simons <jon@jonsimons.org> writes:

> +A bugfix patch should include tests in test_expect_success() blocks that
> +demonstrate the breakage when the code change that implements the fix is
> +omitted.  Submit tests as `test_expect_success` in the same commit as
> +your code change that fixes the bug.  A separate commit that only changes
> +`test_expect_failure` to `test_expect_success` can leave the behavior
> +being tested out of the diff context, making the fix harder to review.
> +Reviewers who want to verify that your test reproduces the bug can
> +temporarily undo the code changes while keeping the test.
> +
>  Pushing to a fork of https://github.com/git/git will use their CI
>  integration to test your changes on Linux, Mac and Windows. See the
>  <<GHCI,GitHub CI>> section for details.

Doesn't the second sentence "Submit tests ..." merely repeat what
the first sentence already says without adding much value?  After
re-reading them three times, I think "the same commit" is the only
new point.  I would condense the first two sentences down to

    A bugfix patch should add, in the same commit, code change and
    test_expect_success() blocks that demonstrate the breakage when
    the fix is omitted.

or something, if I were writing this from scratch.

By the way, I consider that the SubmittingPatches document is
primarily about giving readers a set of rules they should follow,
and not about teaching the reasoning behind a rule to such a deep
level, unless the rule requires judgment on when or when not to
apply it.  We agreed we do not need "unless there is a strong reason
not to", which means there is no judgment needed, so I was not
expecting to see the latter half of the paragraph.

Having said all that, i will queue this version as-is because we are
quickly reaching the point of diminishing returns.  Thanks.