Message ID | 1422294141-16643-1-git-send-email-ptyser@xes-inc.com |
---|---|
State | Accepted |
Delegated to: | Simon Glass |
Headers | show |
Hi Peter, On 26 January 2015 at 10:42, Peter Tyser <ptyser@xes-inc.com> wrote: > When run with the --dry-run argument patman prints out information > showing what it would do. This information currently doesn't line up > with what patman/git send-email really do. Some basic examples: > - If an email address is addressed via "Series-cc" and "Patch-cc" patman > shows that email address would be CC-ed two times. > - If an email address is addressed via "Series-to" and "Patch-cc" patman > shows that email address would be sent TO and CC-ed. > - If an email address is addressed from a combination of tag aliases, > get_maintainer.pl output, "Series-cc", "Patch-cc", etc patman shows > that the email address would be CC-ed multiple times. > > Patman currently does try to send duplicate emails like the --dry-run > output shows, but "git send-email" intelligently removes duplicate > addresses so this patch shouldn't change the non-dry-run functionality. > > Change patman's output and email addressing to line up with the > "git send-email" logic. This trims down patman's dry-run output and > prevents confusion about what patman will do when emails are actually > sent. Thanks for the patch, it's good to match up with git send-email. Are the rules that git send-email follows documented or obtained by trial and error? Regards, Simon
On Mon, 2015-01-26 at 22:21 -0700, Simon Glass wrote: > Hi Peter, > > On 26 January 2015 at 10:42, Peter Tyser <ptyser@xes-inc.com> wrote: > > When run with the --dry-run argument patman prints out information > > showing what it would do. This information currently doesn't line up > > with what patman/git send-email really do. Some basic examples: > > - If an email address is addressed via "Series-cc" and "Patch-cc" patman > > shows that email address would be CC-ed two times. > > - If an email address is addressed via "Series-to" and "Patch-cc" patman > > shows that email address would be sent TO and CC-ed. > > - If an email address is addressed from a combination of tag aliases, > > get_maintainer.pl output, "Series-cc", "Patch-cc", etc patman shows > > that the email address would be CC-ed multiple times. > > > > Patman currently does try to send duplicate emails like the --dry-run > > output shows, but "git send-email" intelligently removes duplicate > > addresses so this patch shouldn't change the non-dry-run functionality. > > > > Change patman's output and email addressing to line up with the > > "git send-email" logic. This trims down patman's dry-run output and > > prevents confusion about what patman will do when emails are actually > > sent. > > Thanks for the patch, it's good to match up with git send-email. > > Are the rules that git send-email follows documented or obtained by > trial and error? Trial and error initially. The git source code lined up with what I saw (see the send_message function in git-send-email.perl). I didn't see the policy documented officially, but what git does makes sense to me: - remove any duplicate addresses in the to: field - remove all the to: addresses from the cc: addresses - remove any duplicate cc: addresses This makes sure each email is only sent to an address one time, with the to: field taking precedence over the cc: field. For a recent NAND patch series it looked like I was going to send Scott 2-4 emails per patch which is why I looked into it. Regards, Peter
On 27 January 2015 at 08:40, Peter Tyser <ptyser@xes-inc.com> wrote: > > On Mon, 2015-01-26 at 22:21 -0700, Simon Glass wrote: >> Hi Peter, >> >> On 26 January 2015 at 10:42, Peter Tyser <ptyser@xes-inc.com> wrote: >> > When run with the --dry-run argument patman prints out information >> > showing what it would do. This information currently doesn't line up >> > with what patman/git send-email really do. Some basic examples: >> > - If an email address is addressed via "Series-cc" and "Patch-cc" patman >> > shows that email address would be CC-ed two times. >> > - If an email address is addressed via "Series-to" and "Patch-cc" patman >> > shows that email address would be sent TO and CC-ed. >> > - If an email address is addressed from a combination of tag aliases, >> > get_maintainer.pl output, "Series-cc", "Patch-cc", etc patman shows >> > that the email address would be CC-ed multiple times. >> > >> > Patman currently does try to send duplicate emails like the --dry-run >> > output shows, but "git send-email" intelligently removes duplicate >> > addresses so this patch shouldn't change the non-dry-run functionality. >> > >> > Change patman's output and email addressing to line up with the >> > "git send-email" logic. This trims down patman's dry-run output and >> > prevents confusion about what patman will do when emails are actually >> > sent. >> >> Thanks for the patch, it's good to match up with git send-email. >> >> Are the rules that git send-email follows documented or obtained by >> trial and error? > > Trial and error initially. The git source code lined up with what I > saw (see the send_message function in git-send-email.perl). I didn't > see the policy documented officially, but what git does makes sense > to me: > - remove any duplicate addresses in the to: field > - remove all the to: addresses from the cc: addresses > - remove any duplicate cc: addresses > > This makes sure each email is only sent to an address one time, > with the to: field taking precedence over the cc: field. > > > For a recent NAND patch series it looked like I was going to send Scott > 2-4 emails per patch which is why I looked into it. Thanks. Acked-by: Simon Glass <sjg@chromium.org> Tested-by: Simon Glass <sjg@chromium.org>
On 27 January 2015 at 22:08, Simon Glass <sjg@chromium.org> wrote: > On 27 January 2015 at 08:40, Peter Tyser <ptyser@xes-inc.com> wrote: >> >> On Mon, 2015-01-26 at 22:21 -0700, Simon Glass wrote: >>> Hi Peter, >>> >>> On 26 January 2015 at 10:42, Peter Tyser <ptyser@xes-inc.com> wrote: >>> > When run with the --dry-run argument patman prints out information >>> > showing what it would do. This information currently doesn't line up >>> > with what patman/git send-email really do. Some basic examples: >>> > - If an email address is addressed via "Series-cc" and "Patch-cc" patman >>> > shows that email address would be CC-ed two times. >>> > - If an email address is addressed via "Series-to" and "Patch-cc" patman >>> > shows that email address would be sent TO and CC-ed. >>> > - If an email address is addressed from a combination of tag aliases, >>> > get_maintainer.pl output, "Series-cc", "Patch-cc", etc patman shows >>> > that the email address would be CC-ed multiple times. >>> > >>> > Patman currently does try to send duplicate emails like the --dry-run >>> > output shows, but "git send-email" intelligently removes duplicate >>> > addresses so this patch shouldn't change the non-dry-run functionality. >>> > >>> > Change patman's output and email addressing to line up with the >>> > "git send-email" logic. This trims down patman's dry-run output and >>> > prevents confusion about what patman will do when emails are actually >>> > sent. >>> >>> Thanks for the patch, it's good to match up with git send-email. >>> >>> Are the rules that git send-email follows documented or obtained by >>> trial and error? >> >> Trial and error initially. The git source code lined up with what I >> saw (see the send_message function in git-send-email.perl). I didn't >> see the policy documented officially, but what git does makes sense >> to me: >> - remove any duplicate addresses in the to: field >> - remove all the to: addresses from the cc: addresses >> - remove any duplicate cc: addresses >> >> This makes sure each email is only sent to an address one time, >> with the to: field taking precedence over the cc: field. >> >> >> For a recent NAND patch series it looked like I was going to send Scott >> 2-4 emails per patch which is why I looked into it. > > Thanks. > > Acked-by: Simon Glass <sjg@chromium.org> > Tested-by: Simon Glass <sjg@chromium.org> Applied to x86/patman and now in mainline.
diff --git a/tools/patman/gitutil.py b/tools/patman/gitutil.py index cc5a55a..c593070 100644 --- a/tools/patman/gitutil.py +++ b/tools/patman/gitutil.py @@ -392,7 +392,8 @@ def EmailPatches(series, cover_fname, args, dry_run, raise_on_error, cc_fname, "Or do something like this\n" "git config sendemail.to u-boot@lists.denx.de") return - cc = BuildEmailList(series.get('cc'), '--cc', alias, raise_on_error) + cc = BuildEmailList(list(set(series.get('cc')) - set(series.get('to'))), + '--cc', alias, raise_on_error) if self_only: to = BuildEmailList([os.getenv('USER')], '--to', alias, raise_on_error) cc = [] diff --git a/tools/patman/series.py b/tools/patman/series.py index b67f870..60ebc76 100644 --- a/tools/patman/series.py +++ b/tools/patman/series.py @@ -94,6 +94,9 @@ class Series(dict): cmd: The git command we would have run process_tags: Process tags as if they were aliases """ + to_set = set(gitutil.BuildEmailList(self.to)); + cc_set = set(gitutil.BuildEmailList(self.cc)); + col = terminal.Color() print 'Dry run, so not doing much. But I would do this:' print @@ -106,24 +109,16 @@ class Series(dict): commit = self.commits[upto] print col.Color(col.GREEN, ' %s' % args[upto]) cc_list = list(self._generated_cc[commit.patch]) - - # Skip items in To list - if 'to' in self: - try: - map(cc_list.remove, gitutil.BuildEmailList(self.to)) - except ValueError: - pass - - for email in cc_list: + for email in set(cc_list) - to_set - cc_set: if email == None: email = col.Color(col.YELLOW, "<alias '%s' not found>" % tag) if email: print ' Cc: ',email print - for item in gitutil.BuildEmailList(self.get('to', '<none>')): + for item in to_set: print 'To:\t ', item - for item in gitutil.BuildEmailList(self.cc): + for item in cc_set - to_set: print 'Cc:\t ', item print 'Version: ', self.get('version') print 'Prefix:\t ', self.get('prefix') @@ -131,7 +126,7 @@ class Series(dict): print 'Cover: %d lines' % len(self.cover) cover_cc = gitutil.BuildEmailList(self.get('cover_cc', '')) all_ccs = itertools.chain(cover_cc, *self._generated_cc.values()) - for email in set(all_ccs): + for email in set(all_ccs) - to_set - cc_set: print ' Cc: ',email if cmd: print 'Git command: %s' % cmd @@ -230,7 +225,7 @@ class Series(dict): if add_maintainers: list += get_maintainer.GetMaintainer(commit.patch) all_ccs += list - print >>fd, commit.patch, ', '.join(list) + print >>fd, commit.patch, ', '.join(set(list)) self._generated_cc[commit.patch] = list if cover_fname:
When run with the --dry-run argument patman prints out information showing what it would do. This information currently doesn't line up with what patman/git send-email really do. Some basic examples: - If an email address is addressed via "Series-cc" and "Patch-cc" patman shows that email address would be CC-ed two times. - If an email address is addressed via "Series-to" and "Patch-cc" patman shows that email address would be sent TO and CC-ed. - If an email address is addressed from a combination of tag aliases, get_maintainer.pl output, "Series-cc", "Patch-cc", etc patman shows that the email address would be CC-ed multiple times. Patman currently does try to send duplicate emails like the --dry-run output shows, but "git send-email" intelligently removes duplicate addresses so this patch shouldn't change the non-dry-run functionality. Change patman's output and email addressing to line up with the "git send-email" logic. This trims down patman's dry-run output and prevents confusion about what patman will do when emails are actually sent. Signed-off-by: Peter Tyser <ptyser@xes-inc.com> --- tools/patman/gitutil.py | 3 ++- tools/patman/series.py | 21 ++++++++------------- 2 files changed, 10 insertions(+), 14 deletions(-)