Thread: [CommitFest App] Feature request -- review e-mail additions
Hello,
Just wanted to suggest two minor mods to the review e-mails
auto-generated by the app:
* Prepend a [review] tag to the e-mail subject ... so that e-mails sent to -hackers will read " [HACKERS]
[review] <patch title goes here>"
* Auto-CC the patch author on this e-mail I guess this should speed up reactions / make communication a bit
more fluid.
Dunno whether it'd be appreciated if I posted a tentative patch ¿?
Thanks,
/ J.L.
José Luis Tallón wrote: > Just wanted to suggest two minor mods to the review e-mails > auto-generated by the app: > > * Prepend a [review] tag to the e-mail subject > ... so that e-mails sent to -hackers will read " [HACKERS] [review] > <patch title goes here>" Changing the subject of an email causes Gmail to break the threads, so anything in that line should be discouraged. -1 from me. I would be happier if the subject of the submission email is kept intact, i.e. not use the patch title that was used in commitfest app but use the one in the email. These often differ. > * Auto-CC the patch author on this e-mail > I guess this should speed up reactions / make communication a bit more > fluid. Yes, strong +1 on this. 3) Have the auto-generated emails insert In-Reply-To headers (and perhaps References). -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
* José Luis Tallón (jltallon@adv-solutions.net) wrote: > * Prepend a [review] tag to the e-mail subject > ... so that e-mails sent to -hackers will read " [HACKERS] > [review] <patch title goes here>" Eh, I'm not against it but not sure it's all that necessary either. > * Auto-CC the patch author on this e-mail > I guess this should speed up reactions / make communication a > bit more fluid. This is almost a requirement, imv, as subsequent comments on the review are frequent and it's a crime if the author misses all of that. Thanks! Stephen
On Wed, Mar 30, 2016 at 1:47 PM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote: > José Luis Tallón wrote: >> Just wanted to suggest two minor mods to the review e-mails >> auto-generated by the app: >> >> * Prepend a [review] tag to the e-mail subject >> ... so that e-mails sent to -hackers will read " [HACKERS] [review] >> <patch title goes here>" > > Changing the subject of an email causes Gmail to break the threads, so > anything in that line should be discouraged. -1 from me. I would be > happier if the subject of the submission email is kept intact, i.e. not > use the patch title that was used in commitfest app but use the one in > the email. These often differ. I entirely agree with Alvaro. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Wed, Mar 30, 2016 at 7:47 PM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
-- José Luis Tallón wrote:
> Just wanted to suggest two minor mods to the review e-mails
> auto-generated by the app:
>
> * Prepend a [review] tag to the e-mail subject
> ... so that e-mails sent to -hackers will read " [HACKERS] [review]
> <patch title goes here>"
Changing the subject of an email causes Gmail to break the threads, so
anything in that line should be discouraged. -1 from me. I would be
happier if the subject of the submission email is kept intact, i.e. not
use the patch title that was used in commitfest app but use the one in
the email. These often differ.
That's what the code tries to do. It will add a Re: if there isn't one already, but otherwise should reuse it. At least that's how it's trying to work - do you have a pointer to an example where it doesn't, so I can investigate if something is off?
> * Auto-CC the patch author on this e-mail
> I guess this should speed up reactions / make communication a bit more
> fluid.
Yes, strong +1 on this.
Ok, that should be easy enough to add. I've stuck it on my TODO list and will try to get it ASAP.
3) Have the auto-generated emails insert In-Reply-To headers (and
perhaps References).
It already does. Both. Again, do you have a pointer to where it doesn't?
On Wed, Apr 6, 2016 at 2:11 PM, Magnus Hagander <magnus@hagander.net> wrote:
On Wed, Mar 30, 2016 at 7:47 PM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:José Luis Tallón wrote:> * Auto-CC the patch author on this e-mail
> I guess this should speed up reactions / make communication a bit more
> fluid.
Yes, strong +1 on this.Ok, that should be easy enough to add. I've stuck it on my TODO list and will try to get it ASAP.
That part was indeed easy, so it's now done.
Magnus Hagander wrote: > On Wed, Mar 30, 2016 at 7:47 PM, Alvaro Herrera <alvherre@2ndquadrant.com> > wrote: > > > José Luis Tallón wrote: > > > > > Just wanted to suggest two minor mods to the review e-mails > > > auto-generated by the app: > > > > > > * Prepend a [review] tag to the e-mail subject > > > ... so that e-mails sent to -hackers will read " [HACKERS] [review] > > > <patch title goes here>" > > > > Changing the subject of an email causes Gmail to break the threads, so > > anything in that line should be discouraged. -1 from me. I would be > > happier if the subject of the submission email is kept intact, i.e. not > > use the patch title that was used in commitfest app but use the one in > > the email. These often differ. > > That's what the code tries to do. It will add a Re: if there isn't one > already, but otherwise should reuse it. At least that's how it's trying to > work - do you have a pointer to an example where it doesn't, so I can > investigate if something is off? No, I don't, I was just stating that José Luis' description seemed wrong to me. I haven't looked carefully at the emails generated from commitfest app, but it seems to work fine already. > > > * Auto-CC the patch author on this e-mail > > > I guess this should speed up reactions / make communication a bit > > > more fluid. > > > > Yes, strong +1 on this. > > Ok, that should be easy enough to add. I've stuck it on my TODO list and > will try to get it ASAP. Thanks for doing this :-) > > 3) Have the auto-generated emails insert In-Reply-To headers (and > > perhaps References). > > It already does. Both. Again, do you have a pointer to where it doesn't? No, works fine AFAICS. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services