Re: [PATCH v3 1/2] git-p4: add test case for "Translation of file content failed" error

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 20/09/15 23:29, Eric Sunshine wrote:
On Sun, Sep 20, 2015 at 5:34 PM, Lars Schneider
<larsxschneider@xxxxxxxxx> wrote:

What is the preferred way by the Git community? Combine patch and
test in one commit or a patch commit followed by a test commit? I
would prefer to have everything in one commit.

If the tests are in a separate patch, Junio seems to prefer adding
them after the problem is fixes; the idea being that tests are added
to ensure that some future change doesn't break the feature, as
opposed to showing that your patch fixes a bug.

Whether or not to combine the fix with the new tests often depends
upon the length of the patches and how easy or hard it is to review
them. In this case, the fix itself is fairly short, but the tests are
slightly lengthy, so there may not be a clear cut answer. As a
reviewer, I tend to prefer smaller patches, however, this situation
doesn't demand it, so use your best judgment.

I think in the past we have a test added that demonstrates the problem, with "test_expect_failure", followed by the fix, which also flips the test to "test_expect_success".

Luke

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]