Junio C Hamano <gitster@xxxxxxxxx> writes: > "Glen Choo via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes: > > Not commenting on the code yet as I am in the middle of today's > integration run, but as I notice a bad pattern being followed, let > me comment before it spreads too widely. > > The "add failing test first and then fix the code with flipping the > test to success" is very much unwelcome. For whoever gets curious > enough (me included when accepting posted patch), it is easy to > revert only the part of the commit outside t/ tentatively to see how > the original code breaks. Keeping the fix and protection of the fix > together will avoid mistakes. The post context of the hunk that > changes test_expect_failure to test_expect_success does not cover > the test script, thereby hiding the body of the test that changes > behaviour while reviewing the patch text, which is another downside. Thanks for voicing this, and sorry. I tried this pattern specifically because I thought it make it easier to review for folks who don't touch t/, but I hadn't considered that the reverse might be preferred.
