All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Øystein Walle" <oystwa@gmail.com>
To: git@vger.kernel.org
Cc: "Øystein Walle" <oystwa@gmail.com>
Subject: [PATCH 0/2] pre-commit hook updates
Date: Tue, 25 Nov 2014 23:51:27 +0100	[thread overview]
Message-ID: <cover.1416953772.git.oystwa@gmail.com> (raw)

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?

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 :)

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.

Øystein Walle (2):
  t7503: use write_script to generate hook scripts
  commit: inform pre-commit that --amend was used

 Documentation/githooks.txt |  3 ++-
 builtin/commit.c           |  3 ++-
 t/t7503-pre-commit-hook.sh | 20 ++++++++++++++------
 3 files changed, 18 insertions(+), 8 deletions(-)

-- 
2.2.0.rc2.23.gca0107e

             reply	other threads:[~2014-11-25 22:51 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-25 22:51 Øystein Walle [this message]
     [not found] ` <cover.1416955873.git.oystwa@gmail.com>
2014-11-25 22:51   ` [PATCH 1/2] t7503: use write_script to generate hook scripts Øystein Walle
2014-11-26  1:25     ` Eric Sunshine
2014-11-26  4:51     ` Jeff King
2014-11-26 18:12       ` Junio C Hamano
2014-11-26 19:03         ` Jeff King
2014-11-26 20:07           ` Junio C Hamano
2014-11-25 22:51   ` [PATCH 2/2] commit: inform pre-commit that --amend was used Øystein Walle
2014-11-26  1:36     ` Eric Sunshine
2014-11-26  1:32 ` [PATCH 0/2] pre-commit hook updates Eric Sunshine
2014-11-26  4:52 ` Jeff King
2014-11-26 18:35   ` Junio C Hamano
2014-11-26 18:56     ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cover.1416953772.git.oystwa@gmail.com \
    --to=oystwa@gmail.com \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.