ability to enforce selecting a mailing list to cc

I think this probably remains too niche and complex to have a place in the upstream.

replies to review comments and the patch should be in the same email

This might be implementable as a Custom Field (i.e., an extension instead of a patch) now, which could reduce the cost of maintaining it. It's possible we could upstream it after T6030.

control over what gets sent - sending everything somebody does in the app that is not relevant to the review is not adding value

T5791 and T4103 are the paths forward for this in the upstream, depending on how much control you want.

In T8336#116839, @epriestley wrote:

ability to enforce selecting a mailing list to cc

I think this probably remains too niche and complex to have a place in the upstream.

replies to review comments and the patch should be in the same email

This might be implementable as a Custom Field (i.e., an extension instead of a patch) now, which could reduce the cost of maintaining it. It's possible we could upstream it after T6030.

control over what gets sent - sending everything somebody does in the app that is not relevant to the review is not adding value

T5791 and T4103 are the paths forward for this in the upstream, depending on how much control you want.

I think I don't understand how that would work yet; the control we need is basically full control over emails:

• we want to decide which emails just do not get sent, because a list will be cc'ed
• we need to be able to basically replace everything that goes into an email (our patch does if (ourSetting) { our-email } else { defaultBehavior }, which is what makes merging so hard if some of the stuff moves)

we want to decide which emails just do not get sent, because a list will be cc'ed

T5791 will let you write global Herald rules for outbound mail, like:

When [all] of these conditions match:
[Sending application][is][Differential]
[Recipients][include any of][mailing list a, mailing list b]
[Transactions][do not include][create, update, comment, accept, reject]

Take these actions:
[Do not deliver this message to][mailing list a, mailing list b]

If you don't need this level of power and the existing Settings → Email Preferences controls are sufficient, you could possibly put the mailing list email address on a user account, then log in with that user and set their preferences.

I don't plan to give mailing lists their own Email Preferences panel, nor to implement any set of controls with a level of power between "Email Preferences" and "Herald".

we need to be able to basically replace everything that goes into an email

At HEAD, most of the default mail behavior is defined by custom fields, which can be disabled and reordered. Disabling a custom field removes that section of the mail. Specifically, you can edit differential.fields in Config to review, reorder, and disable fields.

These sections are currently not driven by custom fields: the actual transaction messages, INLINE COMMENTS, CHANGED PRIOR TO COMMIT, REVISION DETAIL, AFFECTED FILES, CHANGE DETAILS.

If you want to remove some of these sections, I'm comfortable with upstreaming changes which move the sections -- other than the main transaction section -- to custom fields, provided the changes themselves aren't an unreasonably hacky mess. For some sections (like REVISION DETAIL), this is trivial. For others, it may not be. This is a path forward to let you remove sections from email via configuration, without maintaining patches.

Similarly, you can add new sections by defining custom fields. These sections are invoked via addCustomFieldsToMailBody() and can live entirely in extension code. See D12997/D13000/D12993 for some recent examples in Diffusion.

This may or may not be helpful/useful in reducing or eliminating how much patch code you maintain, but I don't plan to make email construction completely modular and we're generally satisfied with how email works from the perspective of the upstream and feedback from all other installs. If you really want to replace the entire email, you can probably maintain a one-line patch which just does return build_my_mail_body_instead($object, $xactions, $body); near the top of DifferentialTransactionEditor->buildMailBody() and put everything else in an extension.

In T8336#116857, @epriestley wrote:

we want to decide which emails just do not get sent, because a list will be cc'ed

T5791 will let you write global Herald rules for outbound mail, like:

When [all] of these conditions match:
[Sending application][is][Differential]
[Recipients][include any of][mailing list a, mailing list b]
[Transactions][do not include][create, update, comment, accept, reject]

Take these actions:
[Do not deliver this message to][mailing list a, mailing list b]

If you don't need this level of power and the existing Settings → Email Preferences controls are sufficient, you could possibly put the mailing list email address on a user account, then log in with that user and set their preferences.

I don't plan to give mailing lists their own Email Preferences panel, nor to implement any set of controls with a level of power between "Email Preferences" and "Herald".

That example sounds like pretty much what we need. That would be awesome.

---

we need to be able to basically replace everything that goes into an email

At HEAD, most of the default mail behavior is defined by custom fields, which can be disabled and reordered. Disabling a custom field removes that section of the mail. Specifically, you can edit differential.fields in Config to review, reorder, and disable fields.

These sections are currently not driven by custom fields: the actual transaction messages, INLINE COMMENTS, CHANGED PRIOR TO COMMIT, REVISION DETAIL, AFFECTED FILES, CHANGE DETAILS.

If you want to remove some of these sections, I'm comfortable with upstreaming changes which move the sections -- other than the main transaction section -- to custom fields, provided the changes themselves aren't an unreasonably hacky mess. For some sections (like REVISION DETAIL), this is trivial. For others, it may not be. This is a path forward to let you remove sections from email via configuration, without maintaining patches.

Similarly, you can add new sections by defining custom fields. These sections are invoked via addCustomFieldsToMailBody() and can live entirely in extension code. See D12997/D13000/D12993 for some recent examples in Diffusion.

This may or may not be helpful/useful in reducing or eliminating how much patch code you maintain, but I don't plan to make email construction completely modular and we're generally satisfied with how email works from the perspective of the upstream and feedback from all other installs. If you really want to replace the entire email, you can probably maintain a one-line patch which just does return build_my_mail_body_instead($object, $xactions, $body); near the top of DifferentialTransactionEditor->buildMailBody() and put everything else in an extension.

The problem with that is again the merging - not in the sense that we get conflicts, but that I've found the "core API" to change regularly (which I understand well from a dev point of view ;) and it's really hard to figure out where to get the data from after something changed...

But I'll play around with the custom fields for email and see whether that gives us what we need first (I'll need to actually look at what patches we still have first)

I don't plan to give mailing lists their own Email Preferences panel

I think everyone got roped into the other tasks, but for completeness: plans changed in T8387 and mailing lists are now "mailing list users", which means administrators have access to this panel and can adjust settings. So this might serve as a reasonable stopgap until T5791.

In T8336#116857, @epriestley wrote:

we want to decide which emails just do not get sent, because a list will be cc'ed

T5791 will let you write global Herald rules for outbound mail, like:

When [all] of these conditions match:
[Sending application][is][Differential]
[Recipients][include any of][mailing list a, mailing list b]
[Transactions][do not include][create, update, comment, accept, reject]

Take these actions:
[Do not deliver this message to][mailing list a, mailing list b]

If you don't need this level of power and the existing Settings → Email Preferences controls are sufficient, you could possibly put the mailing list email address on a user account, then log in with that user and set their preferences.

I don't plan to give mailing lists their own Email Preferences panel, nor to implement any set of controls with a level of power between "Email Preferences" and "Herald".

---

we need to be able to basically replace everything that goes into an email

At HEAD, most of the default mail behavior is defined by custom fields, which can be disabled and reordered. Disabling a custom field removes that section of the mail. Specifically, you can edit differential.fields in Config to review, reorder, and disable fields.

These sections are currently not driven by custom fields: the actual transaction messages, INLINE COMMENTS, CHANGED PRIOR TO COMMIT, REVISION DETAIL, AFFECTED FILES, CHANGE DETAILS.

I'm currently playing around with this (as integrating broke our minimal email mode in a non-trivial way). We're getting very close to being able to get rid of most of our customizations - there are currently 2 things that I can't switch off (or I'm missing something):

• the actions at the top of the mail (never want to see those)
• the email prefs at the bottom (for people on the mailing list who don't have a phab account those are unnecessary)

The one change I needed was to special-case empty headers in text sections, with that, the emails start looking pretty nice.

In T8336#124234, @klimek wrote:

In T8336#116857, @epriestley wrote:

we want to decide which emails just do not get sent, because a list will be cc'ed

T5791 will let you write global Herald rules for outbound mail, like:

When [all] of these conditions match:
[Sending application][is][Differential]
[Recipients][include any of][mailing list a, mailing list b]
[Transactions][do not include][create, update, comment, accept, reject]

Take these actions:
[Do not deliver this message to][mailing list a, mailing list b]

If you don't need this level of power and the existing Settings → Email Preferences controls are sufficient, you could possibly put the mailing list email address on a user account, then log in with that user and set their preferences.

I don't plan to give mailing lists their own Email Preferences panel, nor to implement any set of controls with a level of power between "Email Preferences" and "Herald".

---

we need to be able to basically replace everything that goes into an email

At HEAD, most of the default mail behavior is defined by custom fields, which can be disabled and reordered. Disabling a custom field removes that section of the mail. Specifically, you can edit differential.fields in Config to review, reorder, and disable fields.

These sections are currently not driven by custom fields: the actual transaction messages, INLINE COMMENTS, CHANGED PRIOR TO COMMIT, REVISION DETAIL, AFFECTED FILES, CHANGE DETAILS.

I'm currently playing around with this (as integrating broke our minimal email mode in a non-trivial way). We're getting very close to being able to get rid of most of our customizations - there are currently 2 things that I can't switch off (or I'm missing something):

• the actions at the top of the mail (never want to see those)
• the email prefs at the bottom (for people on the mailing list who don't have a phab account those are unnecessary)

Just found out this is already doable by reading the code. Apparently you violated your own rules of "don't add an option" there ;)

A related request from FreeBSD is the ability to add arbitrary email addresses to the CC.