On Tue, Nov 25, 2014 at 5:51 PM, Øystein Walle <oystwa@xxxxxxxxx> wrote: > The first patch changes t/t7503-pre-commit-hook.sh to use write_script > everywhere, as was suggested by Jeff King in the discussion of the > previous patch. > > The second patch is v2 of the patch I sent earlier. I've incorporated > Eric Sunshine's suggestions. I didn't do enough digging; I found > test_expect_failure and assumed this was test_expect_success's twin > brother, but it marked stuff as known breakages so I went with the '!'. > I also found it a bit strange that test_must_fail has a different > signature (to the extent a shell function has one at all). Is my use of > test_must_fail correct? Your use is not correct (as I'll explain when responding to the patch). > I agree with Junio Hamano that it's better to provide no argument at all > rather than an empty one. I also agree with Jeff King that "noamend" is > better than an empty argument. I went with the second one since Jeff > seemed to get the last word :) For what it's worth (probably not much), I agree with Junio. > I'm not sure I like the ternary inside the function call like that, but > I went with it because it gave the smallest footprint (which is probably > not a good argument). I suppose I could have done: > > if (amend) > hook_arg1 = "amend" > else > hook_arg1 = "noamend" > ... > ... run_commit_hook(use_editor, index_file, "pre-commit", hook_arg1, NULL); > > or create a hook_amend variable. > > I'm happy to send a v3. -- 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