threads / patch / 34431

v4, 2 partsgit-multimail: a replacement for post-receive-email

Subject: [PATCH v4 0/2] git-multimail: a replacement for post-receive-email

## tl;dr

8 messages between Jul 14, 2013 and Jul 23, 2013. Diffs are folded; open one to read it.

replies: 7people: 4as markdown or json

Michael Haggerty· Jul 14, 2013, 08:09 UTC · lore

This is the fourth iteration submission of git-multimail to Git. The earlier submissions have all gotten a lot of good feedback, which has mostly been implemented. This submission differs from v3 in the following ways:

* Renames the directory within the Git project from
  contrib/hooks/git-multimail/ to contrib/hooks/multimail/.  The
  "git-" seems redundant within the Git project [1].  I have no strong
  feelings either way.
* Includes the latest version of the upstream project.  Highlights:
  * A change in how git-multimail is configured via Python.  I wasn't
    happy with the old method; concerns were not separated well
    enough.  I wanted to get this right before more people start
    writing against the internal API.  The new version uses mixin
    classes, which is a technique that can easily be overdone.  But in
    this case I was happy with the way that it permitted different
    aspects of the configuration to be disentangled quite well.  (This
    doesn't change how the script is configured externally via "git
    config"; that has been stable for quite some time already.)
  * Fixes a scalability issue for repos with lots of refs that was
    pointed out by Ævar Bjarmason.
  * Allows an arbitrary program to be substituted in place of
    /usr/sbin/sendmail when using the SendMailer.
  * Improvements suggested by Ramkumar Ramachandra's code review
    (thanks!).
  * Various documentation improvements.
* Adds a new file, README.Git, which explains the relationship between
  the Git and git-multimail projects, and documents the version of the
  upstream project that corresponds to the code being submitted to the
  Git project.
* Adds a notice to contrib/hooks/multimail/post-receive deprecating
  that script and pointing users to git-multimail.

The upstream project also now includes better tests. Though I am not including the tests in the code submitted to the Git project, obviously the code benefits from them.

[1] I also got the feeling that Junio prefers the new directory name,
    though there is a good chance that I read more into one of his
    emails than he intended.
Michael Haggerty (2):
  git-multimail: an improved replacement for post-receive-email
  post-receive-email: deprecate script in favor of git-multimail
 contrib/hooks/multimail/README                     |  486 ++++
 contrib/hooks/multimail/README.Git                 |   15 +
 .../README.migrate-from-post-receive-email         |  146 ++
 contrib/hooks/multimail/git_multimail.py           | 2394 ++++++++++++++++++++
 contrib/hooks/multimail/migrate-mailhook-config    |  270 +++
 contrib/hooks/multimail/post-receive               |   90 +
 contrib/hooks/post-receive-email                   |   17 +-
 7 files changed, 3414 insertions(+), 4 deletions(-)
 create mode 100644 contrib/hooks/multimail/README
 create mode 100644 contrib/hooks/multimail/README.Git
 create mode 100644 contrib/hooks/multimail/README.migrate-from-post-receive-email
 create mode 100755 contrib/hooks/multimail/git_multimail.py
 create mode 100755 contrib/hooks/multimail/migrate-mailhook-config
 create mode 100755 contrib/hooks/multimail/post-receive
-- 
1.8.3.2
Michael Haggerty· Jul 14, 2013, 08:09 UTC · re: Michael Haggerty · lore

[PATCH v4 2/2] post-receive-email: deprecate script in favor of git-multimail

Add a notice to the top of post-receive-email explaining that the script is no longer under active development and pointing the user to git-multimail.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
If you think this is premature or the wording is too strong; feel free
to change or omit this patch.  I think it's fine, but then I might be
biased...
 contrib/hooks/post-receive-email | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)
Show changes to contrib/hooks/post-receive-email +13 −4
diff --git a/contrib/hooks/post-receive-email b/contrib/hooks/post-receive-email
index 0e5b72d..23d7783 100755
--- a/contrib/hooks/post-receive-email
+++ b/contrib/hooks/post-receive-email
@@ -2,10 +2,19 @@
 #
 # Copyright (c) 2007 Andy Parkins
 #
-# An example hook script to mail out commit update information.  This hook
-# sends emails listing new revisions to the repository introduced by the
-# change being reported.  The rule is that (for branch updates) each commit
-# will appear on one email and one email only.
+# An example hook script to mail out commit update information.
+#
+# ***NOTICE***: This script is no longer under active development.  It
+# has been superseded by git-multimail, which is more capable and
+# configurable and is largely backwards-compatible with this script;
+# please see "contrib/hooks/multimail/".  For instructions on how to
+# migrate from post-receive-email to git-multimail, please see
+# "README.migrate-from-post-receive-email" in that directory.
+#
+# This hook sends emails listing new revisions to the repository
+# introduced by the change being reported.  The rule is that (for
+# branch updates) each commit will appear on one email and one email
+# only.
 #
 # This hook is stored in the contrib/hooks directory.  Your distribution
 # will have put this somewhere standard.  You should make this script

-- 
1.8.3.2
Jonathan Nieder· Jul 15, 2013, 06:02 UTC · re: Michael Haggerty · lore

Re: [PATCH v4 2/2] post-receive-email: deprecate script in favor of git-multimail

Michael Haggerty wrote:
> Add a notice to the top of post-receive-email explaining that the
> script is no longer under active development and pointing the user to
> git-multimail.
I think the spirit of this patch is sane.  Some thoughts on wording:
[...]
Show 18 quoted lines
> --- a/contrib/hooks/post-receive-email
> +++ b/contrib/hooks/post-receive-email
> @@ -2,10 +2,19 @@
>  #
>  # Copyright (c) 2007 Andy Parkins
>  #
> -# An example hook script to mail out commit update information.  This hook
> -# sends emails listing new revisions to the repository introduced by the
> -# change being reported.  The rule is that (for branch updates) each commit
> -# will appear on one email and one email only.
> +# An example hook script to mail out commit update information.
> +#
> +# ***NOTICE***: This script is no longer under active development.  It
> +# has been superseded by git-multimail, which is more capable and
> +# configurable and is largely backwards-compatible with this script;
> +# please see "contrib/hooks/multimail/".  For instructions on how to
> +# migrate from post-receive-email to git-multimail, please see
> +# "README.migrate-from-post-receive-email" in that directory.
I think I'd say something like
(1)
	# An example hook script to mail out commit update information.
	#
	# This script is kept for compatibility, but it is no longer actively
	# maintained.  Consider switching to git-multimail, which is more
	# configurable and largely compatible with this script.  See
	# contrib/hooks/multimail/README.migrate-from-post-receive.
	#
	# This hook sends emails listing new revisions ...
or, if I wanted to emphasize the warning,
(2)
	# An example hook ...
	#
	# Warning: this script is kept for compatibility, but it is no longer
	# actively maintained.  Consider switching to ...

or, if I wanted to avoid seeming to promise that the script will be around in the future,

(3)
	# An example hook ...
	#
	# Warning: this script is no longer actively maintained.  Consider
	# switching to ...

I prefer (2), which makes it clear to the reader that it is dangerous to keep using the script (since no one is actively chasing down bugs) while also making it clear why a potentially buggy script with a good natural successor is still in contrib for now. What do you think?

Thanks, Jonathan

Michael Haggerty· Jul 15, 2013, 08:17 UTC · re: Jonathan Nieder · lore

Re: [PATCH v4 2/2] post-receive-email: deprecate script in favor of git-multimail

On 07/15/2013 08:02 AM, Jonathan Nieder wrote:
Show 61 quoted lines
> Michael Haggerty wrote:
> 
>> Add a notice to the top of post-receive-email explaining that the
>> script is no longer under active development and pointing the user to
>> git-multimail.
> 
> I think the spirit of this patch is sane.  Some thoughts on wording:
> 
> [...]
>> --- a/contrib/hooks/post-receive-email
>> +++ b/contrib/hooks/post-receive-email
>> @@ -2,10 +2,19 @@
>>  #
>>  # Copyright (c) 2007 Andy Parkins
>>  #
>> -# An example hook script to mail out commit update information.  This hook
>> -# sends emails listing new revisions to the repository introduced by the
>> -# change being reported.  The rule is that (for branch updates) each commit
>> -# will appear on one email and one email only.
>> +# An example hook script to mail out commit update information.
>> +#
>> +# ***NOTICE***: This script is no longer under active development.  It
>> +# has been superseded by git-multimail, which is more capable and
>> +# configurable and is largely backwards-compatible with this script;
>> +# please see "contrib/hooks/multimail/".  For instructions on how to
>> +# migrate from post-receive-email to git-multimail, please see
>> +# "README.migrate-from-post-receive-email" in that directory.
> 
> I think I'd say something like
> 
> (1)
> 	# An example hook script to mail out commit update information.
> 	#
> 	# This script is kept for compatibility, but it is no longer actively
> 	# maintained.  Consider switching to git-multimail, which is more
> 	# configurable and largely compatible with this script.  See
> 	# contrib/hooks/multimail/README.migrate-from-post-receive.
> 	#
> 	# This hook sends emails listing new revisions ...
> 
> or, if I wanted to emphasize the warning,
> 
> (2)
> 	# An example hook ...
> 	#
> 	# Warning: this script is kept for compatibility, but it is no longer
> 	# actively maintained.  Consider switching to ...
> 
> or, if I wanted to avoid seeming to promise that the script will be
> around in the future,
> 
> (3)
> 	# An example hook ...
> 	#
> 	# Warning: this script is no longer actively maintained.  Consider
> 	# switching to ...
> 
> I prefer (2), which makes it clear to the reader that it is dangerous
> to keep using the script (since no one is actively chasing down bugs)
> while also making it clear why a potentially buggy script with a good
> natural successor is still in contrib for now.  What do you think?

Honestly, as the main author of git-multimail, I am biased and think it would be better for other people to make the decision about if/how/when to annotate post-receive-email. Any of your suggestions are fine with me.

Michael
-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
Matthieu Moy· Jul 15, 2013, 08:29 UTC · re: Jonathan Nieder · lore

Re: [PATCH v4 2/2] post-receive-email: deprecate script in favor of git-multimail

Jonathan Nieder <jrnieder@gmail.com> writes:
Show 10 quoted lines
> (3)
> 	# An example hook ...
> 	#
> 	# Warning: this script is no longer actively maintained.  Consider
> 	# switching to ...
>
> I prefer (2), which makes it clear to the reader that it is dangerous
> to keep using the script (since no one is actively chasing down bugs)
> while also making it clear why a potentially buggy script with a good
> natural successor is still in contrib for now.  What do you think?

I don't think it is dangerous to keep using the old script. If you look at its history, it's pretty stable these day. I think it has known bugs in new revision detections that are fixed by git-multimail, but nothing really blocking IMHO.

There are two good reasons to use it: 1) you already use it, and you're too lazy to change (e.g. because it's packaged by Debian and is already there on your server), and 2) you don't have Python on your server.

I think the notice still deserve the "***NOTICE***" or whatever makes it visible enough to distinguish it from the traditional licence & non-warranty header, but I don't think we should kill the old script too early.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
Junio C Hamano· Jul 15, 2013, 13:38 UTC · re: Matthieu Moy · lore

Re: [PATCH v4 2/2] post-receive-email: deprecate script in favor of git-multimail

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
Show 21 quoted lines
> Jonathan Nieder <jrnieder@gmail.com> writes:
>
>> (3)
>> 	# An example hook ...
>> 	#
>> 	# Warning: this script is no longer actively maintained.  Consider
>> 	# switching to ...
>>
>> I prefer (2), which makes it clear to the reader that it is dangerous
>> to keep using the script (since no one is actively chasing down bugs)
>> while also making it clear why a potentially buggy script with a good
>> natural successor is still in contrib for now.  What do you think?
>
> I don't think it is dangerous to keep using the old script. If you look
> at its history, it's pretty stable these day. I think it has known bugs
> in new revision detections that are fixed by git-multimail, but nothing
> really blocking IMHO.
>
> There are two good reasons to use it: 1) you already use it, and you're
> too lazy to change (e.g. because it's packaged by Debian and is already
> there on your server), and 2) you don't have Python on your server.
Well said.
> I think the notice still deserve the "***NOTICE***" or whatever makes it
> visible enough to distinguish it from the traditional licence &
> non-warranty header, but I don't think we should kill the old script too
> early.

True. I personally felt that Jonathan's (1) read the most natural (i.e. showing no strong preference, just let the users decide).

Junio C Hamano· Jul 23, 2013, 04:20 UTC · re: Junio C Hamano · lore

Re: [PATCH v4 2/2] post-receive-email: deprecate script in favor of git-multimail

Junio C Hamano <gitster@pobox.com> writes:
Show 7 quoted lines
>> I think the notice still deserve the "***NOTICE***" or whatever makes it
>> visible enough to distinguish it from the traditional licence &
>> non-warranty header, but I don't think we should kill the old script too
>> early.
>
> True.  I personally felt that Jonathan's (1) read the most natural
> (i.e. showing no strong preference, just let the users decide).

We need a decider to push this forward, so let's squash this in to tone it down and then merge the result to 'next' (and then 'master' by -rc1).

 contrib/hooks/post-receive-email | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Show changes to contrib/hooks/post-receive-email +2 −2
diff --git a/contrib/hooks/post-receive-email b/contrib/hooks/post-receive-email
index 23d7783..1531150 100755
--- a/contrib/hooks/post-receive-email
+++ b/contrib/hooks/post-receive-email
@@ -4,8 +4,8 @@
 #
 # An example hook script to mail out commit update information.
 #
-# ***NOTICE***: This script is no longer under active development.  It
-# has been superseded by git-multimail, which is more capable and
+# NOTE: This script is no longer under active development.  There
+# is another script, git-multimail, which is more capable and
 # configurable and is largely backwards-compatible with this script;
 # please see "contrib/hooks/multimail/".  For instructions on how to
 # migrate from post-receive-email to git-multimail, please see
Jonathan Nieder· Jul 15, 2013, 05:47 UTC · lore

Re: [PATCH v4 1/2] git-multimail: an improved replacement for post-receive-email

Michael Haggerty wrote:
Show 5 quoted lines
> Add git-multimail, a tool for generating notification emails for
> pushes to a Git repository.  It is largely plug-in compatible with
> post-receive-email, and is proposed to eventually replace that script.
> The advantages of git-multimail relative to post-receive-email are
> described in README.migrate-from-post-receive-email.
Yay!
Show 13 quoted lines
>  contrib/hooks/multimail/README                     |  486 ++++
>  contrib/hooks/multimail/README.Git                 |   15 +
>  .../README.migrate-from-post-receive-email         |  146 ++
>  contrib/hooks/multimail/git_multimail.py           | 2394 ++++++++++++++++++++
>  contrib/hooks/multimail/migrate-mailhook-config    |  270 +++
>  contrib/hooks/multimail/post-receive               |   90 +
>  6 files changed, 3401 insertions(+)
>  create mode 100644 contrib/hooks/multimail/README
>  create mode 100644 contrib/hooks/multimail/README.Git
>  create mode 100644 contrib/hooks/multimail/README.migrate-from-post-receive-email
>  create mode 100755 contrib/hooks/multimail/git_multimail.py
>  create mode 100755 contrib/hooks/multimail/migrate-mailhook-config
>  create mode 100755 contrib/hooks/multimail/post-receive

I wouldn't be surprised if it is possible to improve this in some way or other, but where as usual "Reviewed-by" means "If I were maintainer I would commit and push it out and as a non maintainer I vouch for its suitability",

Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

The patch integrates it into the git tree sanely and further refinements can safely come on top. Thanks for your hard work.

← back to recent threads