On Tue, Aug 29, 2017 at 6:37 AM, Philipp Tomsich <philipp.toms...@theobroma-systems.com> wrote: > > > On Mon, 28 Aug 2017, Chris Packham wrote: > >> Add a --bounce-file option which can be used to omit addresses that are >> known to bounce. The option specifies a file which lists addresses (one >> per line) that are stripped from the Cc list. > > > Reviewed-by: Philipp Tomsich <philipp.toms...@theobroma-systems.com> > > Absolutely love this (in fact, I had something like this on my to-do list as > the number of bounces seemed to grow over time). > > A few comments below. > >> >> Signed-off-by: Chris Packham <judge.pack...@gmail.com> >> --- >> Here's a quick proof of concept. Right now the .bounces file has to >> match exactly the addresses that are extracted by patman. This makes it >> easy to do set(a) - set(b) to remove all the bounces in one hit. >> >> tools/patman/patman.py | 6 +++++- >> tools/patman/series.py | 5 ++++- >> tools/patman/tools.py | 8 ++++++++ >> 3 files changed, 17 insertions(+), 2 deletions(-) >> >> diff --git a/tools/patman/patman.py b/tools/patman/patman.py >> index 4b3bc787453e..567226a4d3ce 100755 >> --- a/tools/patman/patman.py >> +++ b/tools/patman/patman.py >> @@ -56,6 +56,9 @@ parser.add_option('-v', '--verbose', >> action='store_true', dest='verbose', >> default=False, help='Verbose output of errors and warnings') >> parser.add_option('--cc-cmd', dest='cc_cmd', type='string', >> action='store', >> default=None, help='Output cc list for patch file (used by git)') >> +parser.add_option('--bounce-file', dest='bounce_fname', type='string', >> + default='.bounces', > > > The '.bounces' default doesn't really fit with how such settings are handled > throughout patman. > > My gut feeling is that > (a) we want to have a project-wide bounces config (doc/git-bounces ???) > that complements doc/git-mailrc > (b) that any overrides/user-local additional settings should be picked up > via ~/.patman (either by referencing a local overrides file or by > having some additional overrides directly there) > (c) this entire mechanism needs to be tied into Setup(...) in settings.py >
Makes sense. The .bounces file was just convenient. But yeah you are likely to have project-wide settings that are stored in-tree and user additions that aren't shared. > >> + help='File containing addresses to skip [default: >> %default]') >> parser.add_option('--no-check', action='store_false', dest='check_patch', >> default=True, >> help="Don't check for patch compliance") >> @@ -158,7 +161,8 @@ else: >> >> cc_file = series.MakeCcFile(options.process_tags, cover_fname, >> not options.ignore_bad_tags, >> - options.add_maintainers) >> + options.add_maintainers, >> + options.bounce_fname) >> >> # Email the patches out (giving the user time to check / cancel) >> cmd = '' >> diff --git a/tools/patman/series.py b/tools/patman/series.py >> index d3947a7c2ac5..b6533ab5ee58 100644 >> --- a/tools/patman/series.py >> +++ b/tools/patman/series.py >> @@ -11,6 +11,7 @@ import os >> import get_maintainer >> import gitutil >> import terminal >> +from tools import FindBounces >> >> # Series-xxx tags that we understand >> valid_series = ['to', 'cc', 'version', 'changes', 'prefix', 'notes', >> 'name', >> @@ -202,7 +203,7 @@ class Series(dict): >> print(col.Color(col.RED, str)) >> >> def MakeCcFile(self, process_tags, cover_fname, raise_on_error, >> - add_maintainers): >> + add_maintainers, bounce_fname): >> """Make a cc file for us to use for per-commit Cc automation >> >> Also stores in self._generated_cc to make ShowActions() faster. >> @@ -222,6 +223,7 @@ class Series(dict): >> fname = '/tmp/patman.%d' % os.getpid() >> fd = open(fname, 'w') >> all_ccs = [] >> + bounces = FindBounces(bounce_fname) >> for commit in self.commits: >> cc = [] >> if process_tags: >> @@ -234,6 +236,7 @@ class Series(dict): >> elif add_maintainers: >> cc += get_maintainer.GetMaintainer(commit.patch) >> cc = [m.encode('utf-8') if type(m) != str else m for m in cc] >> + cc = set(cc) - set(bounces) >> all_ccs += cc >> print(commit.patch, ', '.join(set(cc)), file=fd) >> self._generated_cc[commit.patch] = cc >> diff --git a/tools/patman/tools.py b/tools/patman/tools.py >> index ba2485303037..9d206a390b02 100644 >> --- a/tools/patman/tools.py >> +++ b/tools/patman/tools.py >> @@ -118,3 +118,11 @@ def Align(pos, align): >> >> def NotPowerOfTwo(num): >> return num and (num & (num - 1)) >> + >> +def FindBounces(bounce_fname): >> + """generate a list of bouncing addresses from a file""" >> + try: >> + with open(bounce_fname) as fd: >> + return [line.strip() for line in fd] >> + except IOError: >> + return [] >> > > Shouldn't this be merged into gitutil.BuildEmailList or into > gitutil.LookupEmail? It would also be nice if the bounces would be handled > in analogy to aliases and referenced via settings.alias I struggled to find a home for this. 'tools.py' was just the first place I found for a reasonably generic helper function. If this code follows the way aliases work it makes sense for it to live there. > > Regards, > Philipp. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot