David Turner <dturner@xxxxxxxxxxxxxxxx> writes: > On Fri, 2016-05-06 at 18:13 +0200, Michael Haggerty wrote: >> Thanks to David, Junio, and Peff for their comments on v1 of this >> patch series [1]. I think I have addressed all of the points that >> were >> brought up. Plus I fixed a pre-existing bug that I noticed myself >> while adding some more tests; see the first bullet point below for >> more information. >> >> Changes between v1 and v2: >> >> * Prefixed the patch series with three new patches that demonstrate >> and fix some bugs in reference resolution that I noticed while >> inspecting this series: >> >> * "t1404: demonstrate a bug resolving references" -- this >> demonstrates a bug that has existed since at least 2.5.0. >> * "read_raw_ref(): don't get confused by an empty directory" >> * "commit_ref(): if there is an empty dir in the way, delete it" > > I generally like to put the bug fixes before the tests for those fixes > (so that bisect on the complete suite works). But maybe the git policy > is different. The Git policy only asks not to break bisection. As long as patch that adds a new test that comes before a patch that fixes the issue marks the new test with test_expect_failure, and a later patch that fixes the issue turns it into test_expect_success, bisection would not break. The "demonstrate an existing breakage first" order makes it slightly easier to review and follow a long series, as it forces the reviewer to see the issue first and think about possible avenues to solve it for themselves, before seeing a paticular solution. For a trivial single-issue fix, it is not necessary (including a fix and a test to protect the fix from future breakage in the same patch is a norm). -- 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