Send emails to both users and mailing lists for review groups.
Review Request #5828 — Created May 14, 2014 and submitted
When we initially created review groups, we made the assumption that all groups
would have an associated mailing list. This made a lot of sense when
transitioning from an email based code review system. Shortly after, we made
it so that any review group that didn't have a mailing list would instead send
email directly to all the members. This creates a confusing situation whereby
users are expected to subscribe to mailing lists if they're present but
otherwise get emails.This change adds an option to review groups to specify whether emails should go
only to the list, or to the list plus every subscriber. The default for this is
to keep the current behavior.
- Tested evolution.
- Verified that with this check-box off, the recipient list for a review
request assigned to a group with a mailing list contained both the mailing
list and the group members.
Description | From | Last Updated |
---|---|---|
Blank line after. |
chipx86 | |
Can we change this to not g.mailing_list or not g.email_list_only? I find it a lot more readable (no mental parsing … |
chipx86 | |
"list" is a bit vague when looking at the page, since there's the mailing list, lists of users, etc. I … |
chipx86 | |
Is it? I'm still not convinced. I think whether it's highly recommended very much depends on the organization. I'd prefer … |
chipx86 |
- Change Summary:
-
Make it optional.
- Description:
-
When we initially created review groups, we made the assumption that all groups
would have an associated mailing list. This made a lot of sense when transitioning from an email based code review system. Shortly after, we made it so that any review group that didn't have a mailing list would instead send email directly to all the members. This creates a confusing situation whereby users are expected to subscribe to mailing lists if they're present but otherwise get emails. ~ Because email clients are pretty darned good at de-duping when lists are
~ involved, and users can already get duplicates when they're directly assigned ~ to something and/or are the sender, I'm changing it from OR to AND. This means ~ This change adds an option to review groups to specify whether emails should go
~ only to the list, or to the list plus every subscriber. The default for this is ~ to keep the current behavior. - that users can go around subscribing to groups, and they don't have to - magically know whether or not there's a mailing list. - Testing Done:
-
~ Verified that the recipient list for a review request assigned to a group
~ with a mailing list contained both the mailing list and the group members. ~ - Tested evolution.
~ - Verified that with this check-box off, the recipient list for a review
request assigned to a group with a mailing list contained both the mailing
list and the group members.
- Branch:
-
release-1.7.xmaster
- Commit:
-
7df0cce75a144551117ad39846e8897fce5af6aa5a74fc498f9fb8e916d8c5d9315b7ca3f797c408
-
This is a review from Review Bot. Tool: PEP8 Style Checker Processed Files: reviewboard/notifications/email.py reviewboard/reviews/models/group.py reviewboard/reviews/evolutions/group_email_list_only.py reviewboard/reviews/evolutions/__init__.py Ignored Files:
-
This is a review from Review Bot. Tool: Pyflakes Processed Files: reviewboard/notifications/email.py reviewboard/reviews/models/group.py reviewboard/reviews/evolutions/group_email_list_only.py reviewboard/reviews/evolutions/__init__.py Ignored Files:
-
-
-
Can we change this to
not g.mailing_list or not g.email_list_only
? I find it a lot more readable (no mental parsing required). -
"list" is a bit vague when looking at the page, since there's the mailing list, lists of users, etc. I know there's help text, but I think the label can be better.
How about "send e-mail only to the mailing list"?
I also want to be consistent with "email" vs. "e-mail".
-
Is it? I'm still not convinced. I think whether it's highly recommended very much depends on the organization. I'd prefer we just nuke that part of the sentence.
-
This is a review from Review Bot. Tool: PEP8 Style Checker Processed Files: reviewboard/notifications/email.py reviewboard/reviews/models/group.py reviewboard/reviews/evolutions/group_email_list_only.py reviewboard/reviews/evolutions/__init__.py Ignored Files: