"Philip Oakley" <philipoak...@iee.org> writes: > From: "Joachim Schmitz" <j...@schmitz-digital.de> > Sent: Monday, January 21, 2013 2:19 PM >> Junio C Hamano wrote: >>> Barry Wardell <barry.ward...@gmail.com> writes: > [...] >>> Thanks for your persistence ;-) As this is a pretty old topic, I'll >>> give two URLs for people who are interested to view the previous >>> threads: >>> >>> http://thread.gmane.org/gmane.comp.version-control.git/192133 >>> http://thread.gmane.org/gmane.comp.version-control.git/192127 >>> >>> You would want to mark it as test_expect_failure in the first patch >>> and then flip it to text_expect_success in the second patch where >>> you fix the breakage? Otherwise, after applying the first patch, >>> the testsuite will break needlessly. >> >> I'd just apply them the other way round, 1st fix the problem, 2nd >> add a test for it > > Isn't it a case of, 1st demonstrate the problem with a test, and then > 2nd fix the problem. > > Those less principled could could simply "fix" a non-existent problem > merely to get themselves into the change log, or worse, even if one > may fix-test under the hood.
For a small/trivial fix, fixing the code and protecting the fix from future breakages by adding tests that expect success in a single commit is the most sensible thing to do. People who are interested, and people who are auditing, can locally revert only the code change to see the new tests fail fairly easily in such a case. For a more involved series, it is easier to demonstrate a breakage by adding tests that expect failure in the first commit, and then in subsequent commits, to fix a class of bugs in the code and flipping expect_failure into expect_success for the tests that the updated code in the commit fixes. For this particular topic, squashing the two patches into a single commit may probably be the more appropriate between the two. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
