Re: [RFC v2] git-multimail: a replacement for post-receive-email

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [RFC v2] git-multimail: a replacement for post-receive-email

From: Matthieu Moy <hidden>
Date: 2016-06-15 22:56:09

Michael Haggerty [off-list ref] writes:
On 02/13/2013 03:56 PM, Matthieu Moy wrote:
quoted
Installation troubles:

I had an old python installation (Red Hat package, and I'm not root),
that did not include the email.utils package, so I couldn't use my
system's python. I found no indication about python version in README,
so I installed the latest python by hand, just to find out that
git-multimail wasn't compatible with Python 3.x. 2to3 can fix
automatically a number of 3.x compatibility issues, but not all of them
so I gave up and installed Python 2.7.
What version of Python was it that caused problems?
Python 2.4.3, installed with RHEL 5.9.
I just discovered that the script wouldn't have worked with Python
2.4, where "email.utils" used to be called "email.Utils".
Indeed, "import email.Utils" works with this Python.
But I pushed a fix to GitHub:

    ddb1796660 Accommodate older versions of Python's email module.
Not sufficient, but I added a pull request that works for me with 2.4.
quoted
@@ -835,6 +837,17 @@ class ReferenceChange(Change):
                 for line in self.expand_lines(NO_NEW_REVISIONS_TEMPLATE):
                     yield line
 
+            if adds and self.showlog:
+                yield '\n'
+                yield 'Detailed log of added commits:\n\n'
+                for line in read_lines(
+                        ['git', 'log']
+                        + self.logopts
+                        + ['%s..%s' % (self.old.commit, self.new.commit,)],
+                        keepends=True,
+                        ):
+                    yield line
+
             # The diffstat is shown from the old revision to the new
             # revision.  This is to show the truth of what happened in
             # this change.  There's no point showing the stat from the
Thanks for the patch.  I like the idea, but I think the implementation
is incorrect.  Your code will not only list new commits but will also
list commits that were already in the repository on another branch
(e.g., if an existing feature branch is merged into master, all of the
commits on the feature branch will be listed).  (Or was that your
intention?)
I did not think very carefully about this case, but the behavior of my
code seems sensible (although not uncontroversial): it's just showing
the detailed log for the same commits as the summary at the top of the
email. I have no personnal preferences.
But even worse, it will fail to list commits that were
added at the same time that a branch was created (e.g., if I create a
feature branch with a number of commits on it and then push it for the
first time).
Right.
Probably the Push object has to negotiate with its constituent
ReferenceChange objects to figure out which one is responsible for
summarizing each of the commits newly added by the push (i.e., the ones
returned by push.get_new_commits(None)).
I updated the pull request with a version that works for new branches,
and takes the list of commits to display from the call to
get_new_commits (which were already there for other purpose). Then, it
essentially calls "git log --no-walk $list_of_sha1s".

This should be better.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [RFC v2] git-multimail: a replacement for post-receive-email

From: Michael Haggerty <hidden>
Date: 2016-06-15 22:56:09

On 02/14/2013 01:55 PM, Matthieu Moy wrote:
Michael Haggerty [off-list ref] writes:
quoted
On 02/13/2013 03:56 PM, Matthieu Moy wrote:
quoted
Installation troubles:

I had an old python installation (Red Hat package, and I'm not root),
that did not include the email.utils package, so I couldn't use my
system's python. I found no indication about python version in README,
so I installed the latest python by hand, just to find out that
git-multimail wasn't compatible with Python 3.x. 2to3 can fix
automatically a number of 3.x compatibility issues, but not all of them
so I gave up and installed Python 2.7.
What version of Python was it that caused problems?
Python 2.4.3, installed with RHEL 5.9.
quoted
I just discovered that the script wouldn't have worked with Python
2.4, where "email.utils" used to be called "email.Utils".
Indeed, "import email.Utils" works with this Python.
quoted
But I pushed a fix to GitHub:

    ddb1796660 Accommodate older versions of Python's email module.
Not sufficient, but I added a pull request that works for me with 2.4.
quoted
quoted
@@ -835,6 +837,17 @@ class ReferenceChange(Change):
                 for line in self.expand_lines(NO_NEW_REVISIONS_TEMPLATE):
                     yield line
 
+            if adds and self.showlog:
+                yield '\n'
+                yield 'Detailed log of added commits:\n\n'
+                for line in read_lines(
+                        ['git', 'log']
+                        + self.logopts
+                        + ['%s..%s' % (self.old.commit, self.new.commit,)],
+                        keepends=True,
+                        ):
+                    yield line
+
             # The diffstat is shown from the old revision to the new
             # revision.  This is to show the truth of what happened in
             # this change.  There's no point showing the stat from the
Thanks for the patch.  I like the idea, but I think the implementation
is incorrect.  Your code will not only list new commits but will also
list commits that were already in the repository on another branch
(e.g., if an existing feature branch is merged into master, all of the
commits on the feature branch will be listed).  (Or was that your
intention?)
I did not think very carefully about this case, but the behavior of my
code seems sensible (although not uncontroversial): it's just showing
the detailed log for the same commits as the summary at the top of the
email. I have no personnal preferences.
I guess it depends a lot on what logopts are used.  If the user
configures logopts to emit full patches, then the repeated reporting of
the same commits would cause a big increase in the bulk of notification
emails.  But if the logopts are set to just emit a brief summary (e.g.,
author and log message), then a bit of repetition might be acceptable.
But since I wouldn't use this feature, I don't personally have a preference.
quoted
But even worse, it will fail to list commits that were
added at the same time that a branch was created (e.g., if I create a
feature branch with a number of commits on it and then push it for the
first time).
Right.
quoted
Probably the Push object has to negotiate with its constituent
ReferenceChange objects to figure out which one is responsible for
summarizing each of the commits newly added by the push (i.e., the ones
returned by push.get_new_commits(None)).
I updated the pull request with a version that works for new branches,
and takes the list of commits to display from the call to
get_new_commits (which were already there for other purpose). Then, it
essentially calls "git log --no-walk $list_of_sha1s".

This should be better.
I will check it out.

Thanks!

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help