Hi, On Sun, Feb 19, 2023 at 3:50 PM Simon Glass <s...@chromium.org> wrote: > > @@ -234,6 +234,48 @@ class Series(dict): > str = 'Change log exists, but no version is set' > print(col.build(col.RED, str)) > > + def GetCcForCommit(self, commit, process_tags, warn_on_error, > + add_maintainers, limit, get_maintainer_script, > + all_skips): > + """Get the email CCs to use with a particular commit > + > + Uses subject tags and get_maintainers.pl script to find people to cc > + on a patch > + > + Args: > + commit (Commit): Commit to process > + process_tags (bool): Process tags as if they were aliases > + warn_on_error (bool): True to print a warning when an alias > fails to > + match, False to ignore it. > + add_maintainers (bool or list of str): Either: > + True/False to call the get_maintainers to CC maintainers > + List of maintainers to include (for testing) > + limit (int): Limit the length of the Cc list (None if no limit) > + get_maintainer_script (str): The file name of the > get_maintainer.pl > + script (or compatible). > + all_skips (set of str): Set of bouncing email address that were > + dropped from the output
It wouldn't hurt to mention that "all_skips" is essentially a return value from this function (this function updates it to include the email addresses that were skipped). > @@ -259,28 +301,18 @@ class Series(dict): > fname = '/tmp/patman.%d' % os.getpid() > fd = open(fname, 'w', encoding='utf-8') > all_ccs = [] > + all_skips = set() > for commit in self.commits: > - cc = [] > - if process_tags: > - cc += gitutil.build_email_list(commit.tags, > - warn_on_error=warn_on_error) > - cc += gitutil.build_email_list(commit.cc_list, > - warn_on_error=warn_on_error) > - if type(add_maintainers) == type(cc): > - cc += add_maintainers > - elif add_maintainers: > - > - cc += get_maintainer.get_maintainer(get_maintainer_script, > - commit.patch) > - for x in set(cc) & set(settings.bounces): > - print(col.build(col.YELLOW, 'Skipping "%s"' % x)) > - cc = list(set(cc) - set(settings.bounces)) > - if limit is not None: > - cc = cc[:limit] > + cc = self.GetCcForCommit(commit, process_tags, warn_on_error, > + add_maintainers, limit, > + get_maintainer_script, all_skips) > all_ccs += cc > print(commit.patch, '\0'.join(sorted(set(cc))), file=fd) > self._generated_cc[commit.patch] = cc > > + for x in sorted(list(all_skips)): Why "sorted(list(all_skips))" and not just "sorted(all_skips)"? Both of the above are nits, so I'm OK w/: Reviewed-by: Douglas Anderson <diand...@chromium.org>
