[PATCH] D46192: Script to match open Phabricator reviews with potential reviewers

Kristof Beyls via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue May 1 06:44:18 PDT 2018


kristof.beyls added a comment.

In https://reviews.llvm.org/D46192#1082214, @philip.pfaffe wrote:

> Do you plan on actually commiting this to the repository, or would you like any feedback nontheless?


I definitely welcome any feedback.
I wasn't sure on committing to the repo, but with the comments coming in and the new feature requests popping up already, it seems like I should prepare to commit it to the repository, so that new features can be developed and shared incrementally. 
As @lebedev.ri  suggested, it probably should go somewhere in llvm/utils. Given there seems to be scope for other reviewing-related scripts, maybe it should go in a separate directory, e.g. llvm/utils/Reviewing.

> One usability nit that I see is `send_emails()`. It would be great to be able to configure the SMTP connection, to disable the feature altogether, and to seperately specify the recipient of that e-mail: I use E-Mail in my local network, but do not allow for any outgoing traffic.

Agreed, more flexibility is needed on the emailing-feature of the script. I assume that the script is also too hard-coded in some other locations as this is really a prototype containing what was needed in my environment.

I hope to find some time next week to look both into finding a good location to commit this script and the email-feature improvements you suggest .


https://reviews.llvm.org/D46192





More information about the llvm-commits mailing list