This is an archive of the discontinued LLVM Phabricator instance.

Make email options of find_interesting_reviews more flexible.
ClosedPublic

Authored by kristof.beyls on Jun 8 2018, 12:07 AM.

Details

Summary

This enables a few requested improvements on the original review of this
script at https://reviews.llvm.org/D46192.

This introduces 2 new command line options:

  • --email-report: This option enables specifying who to email the generated report to. This also enables not sending any email and only printing out the report on stdout by not specifying this option on the command line.
  • --sender: this allows specifying the email address that will be used in the "From" email header.

I believe that with these options the script starts having the basic
features needed to run it well on a regular basis for a group of
developers.

Diff Detail

Repository
rL LLVM

Event Timeline

kristof.beyls created this revision.Jun 8 2018, 12:07 AM
philip.pfaffe added inline comments.Jun 11 2018, 2:45 AM
utils/Reviewing/find_interesting_reviews.py
591 ↗(On Diff #150452)

Instead of taking and parsing comma separated values, why not use nargs='+'.

On the other hand, it'd be nice to rename the option to '--email-report', which optionally takes email addresses. If none are given, just send it to the email addresses passed to the email_addresses option.

JDevlieghere accepted this revision.Jun 11 2018, 8:52 AM

LGTM with Philip's comment addressed.

utils/Reviewing/find_interesting_reviews.py
591 ↗(On Diff #150452)

+1

This revision is now accepted and ready to land.Jun 11 2018, 8:52 AM
kristof.beyls edited the summary of this revision. (Show Details)
kristof.beyls marked 2 inline comments as done.Jun 28 2018, 7:46 AM
kristof.beyls added inline comments.
utils/Reviewing/find_interesting_reviews.py
591 ↗(On Diff #150452)

Thanks for those suggestions, @philip.pfaffe ! I've also taken your suggestion to rename the option to '--email-report', but went for "When the --email-report flag is not specified, do not send out any emails", to make sure it remains possible to run the script and get a report without sending it over email.

This revision was automatically updated to reflect the committed changes.