Page MenuHomePhabricator

When emails are sent to new people (or lists) they should contain the change description and patch
Closed, WontfixPublic

Description

Currently, when I cc' a new list or add a reviewer the email sent out does contain neither the change description nor the patch - thus, the new recipient is left in the dark about what's going on.
One solution might be to re-send the initial email to all newly added users, and then send the update to all users that are on the review thread afterwards?

Event Timeline

klimek raised the priority of this task from to Needs Triage.
klimek updated the task description. (Show Details)
klimek added a subscriber: klimek.

Another idea of how to structure this would be to have the email when new receipients are added contain some boiler plate at the bottom:


FYI, for new folks on the thread, here is the patch we are discussing:

<description>

<attach patch>

chad triaged this task as Wishlist priority.Nov 2 2014, 11:08 PM
epriestley claimed this task.
epriestley added a subscriber: epriestley.

I don't plan to change this without a better understanding of what problem we're solving. New users can click the link in the email to review the discussion, summary, and patch.

Email is intended to describe state changes, not send you a screenshot of the webpage. It's not clear to me what user behavior is motivating requests that are essentially "I want a screenshot of the page in email" since access to the current object state (in a rich, up-to-date format) is one click away. For example, this problem description seems like it is clearly missing some kind of context:

the new recipient is left in the dark about what's going on.

...since a user who isn't sure what's going on can just click the link in the email to figure it out.

Maybe there's some critical context missing, like "I airgap all my email" or "I read email on a 1999 Nokia that doesn't have a browser" or something. These are good reasons why this is a problem, but these user behaviors are wildly atypical and not things I plan to support in the upstream. The software is open source and you're free to modify it if you have some absurd bizarre workflow which no normal person would choose willingly.

Maybe the problem is some compliance issue, or "Phabricator is VPN'd so I can't click stuff from my phone". These are good reasons why this is a problem, but I'm very hesitant to include VPN/compliance-evasion features in the upstream. Instead, work with your network administrator to improve policy if this is an important part of your workflow. They have configured the network to prevent you from doing this, and their job is to control network policy, so maybe they have a better reason than you do for why this isn't allowed. If your best way forward is really evading your organization's network policy, do it yourself rather than asking us to help you.

Maybe the problem is "I don't like clicking things". I don't think this is a good reason why this is a problem. Instead, click the thing.

Upshot: any request in this class needs a very clear, very strong justification for why clicking links isn't a good way to get more information.

For the record, while the rationale can be flippantly described as "I don't like clicking things", I'd like to give it a serious description.

We have a pretty significant set of contributors to LLVM who have a highly optimized email workflow. This often involves non-web email clients. They triage code reviews at a high rate of productivity in this workflow. They don't have a web browser in use when doing this. Starting a web browser may not be fast or efficient, and is empirically enough for them to simply skip the review.

However, we also have many users who found Phab to be a *dramatic* increase in productivity for code review. Unfortunately, without emails that don't require clicking through, we have a big barrier to adoption and a terrible choice: either provide a good review experience to group #1 or to group #2, you cannot provide it to both.

The end result of this dilemma was that we were unable to adopt anything *other* than email for code review for many, many years. We fixed this by heavily customizing the LLVM Phabricator instance to generate emails that were as good for the email review flow of old, and still contained a link to a proper web interface for the second group. Both groups could now review emails. This was demonstrably successful -- LLVM now *heavily* relies on this tool for reviews! This has made us a big fan of Phab all around. However, there were some significant pain points left, and this issue describes one of them.

Now, it is totally reasonable to say "look, we're never going to support this use case". And if that is the direction, then sure, you can just ignore this issue. However, I will point out that I think this is a significant error. This is much like building a developer tool for C++ (something we also do a lot of) but only supporting rich, modern IDEs. That does simplify everything. That may be the workflow you *want* people to adopt. But it kills incremental adoption. The idea of "we'll keep stacking up awesome on the grand new way to do things" is very tempting to get everyone to switch. However, in my experience, it simply results in the "jump" being too large, and everyone working around their lack of awesome functionality. I would much rather make as much awesome available in every flow, and then make adopting the next chunk "one click away".

Put differently, our goal is to get people to "click the thing". But *failing* to put the information in the email doesn't make that more likely, it makes it less likely. Instead, if we don't give them the best email we can, with the maximum of information we can put there, we simply decrease the chance they even *read* the email, instead moving along to something more interesting. There are always more things to do, and code review is often not everyone's favorite to do. So I think the right approach is to make every way in which users interact with the code review system as helpful and featureful as possible. The complexity it places on the code review system itself seems minor compared to the impact on *causing reviews to happen*.

Anyways, that's my soap box and two cents. I understand if it just doesn't convince people. I wish it would, because I don't know what else will *really* drive adoption for the far more numerous than I might wish email readers using a non-browser email experience.

I think a lot of this behavior may be fairly unique to the LLVM project, since we've never really received this general class of feedback from other installs (except, perhaps, FreeBSD, which I think is somewhat culturally similar).

The closest other installs have come is generally feedback along the lines of "I want to be able to passively follow along with all changes from Gmail without clicking anything", which motivated, e.g., inline comment improvements in T10694 to support "passively follow from a mail client" and partially motivated better mail representation of text diffs (e.g., updating a summary or test plan). I generally don't recall reports of "actively engage in review only through a mail client" from other installs, unless there was a technical caveat like "so I can evade the VPN". My sense is that almost all usage of email !commands like !accept is occasional (quick interactions from mobile) or using the workflow as a convenient API for bots (which we generally plan to expand in T11934).

Alongside the inline comment improvements I ripped a substantial LLVM patch for inlines out of the upstream (T10694#196860) and feedback since then has all been positive with no (?) requests for the feature to return, so I believe that specific feature (non-HTML email comments with threaded text context) is pretty exclusively valuable to LLVM. While I can certainly appreciate that there is a class of user who prefers this:

Screen Shot 2019-01-02 at 10.03.23 AM.png (201×1 px, 36 KB)

...to this:

Screen Shot 2019-01-02 at 10.03.39 AM.png (220×1 px, 41 KB)

...I think they may be somewhat far removed from modern mainstream development.

Practically, essentially all of our customers develop web or mobile software and I believe very few of their engineers use a non-browser workflow to interact with mail. Particularly, we've seen far more feedback requesting better support specifically for the GMail web client (e.g., workarounds for peculiar filtering/rule behavior) because it is so ubiquitous than requests for improving behavior in any other client. (I use a non-web mail client exclusively and don't like web mail, but I click links from it.)

If this kind of flow was more common among customers I'd be more open to considering more features in the general vein of "take a screenshot of the webpage, convert it into plain text, and email it" but even customers writing in low-level languages today are generally ultimately targeting web or mobile UIs at top-level.

Some planned features (like T11934, mentioned above) will improve CLI and email access to Phabricator, but features that are philosophically "email describes state" instead of "email describes changes" tend to create a lot of complexity and product conflicts (see also T8227, and many complaints that email is "too big" or "too hard to read" if we try to simply include more information). Without much stronger customer interest in "email describes state" use cases, I think they're best served today by forking (as with the existing LLVM inline patch).

In the modern codebase, the proposed change here could likely be accomplished by making applyOldRecipientLists() track mailAddedPHIDs, similar to mailRemovedPHIDs, then sending them a separate "you're about to be added to the thread" email at the last moment. This is probably a substantial change to get right in all cases (e.g., making it thread properly). Sending them an actual byte-for-byte copy of the original email is likely not appropriate (the patch and/or description may have changed). But generating an approximately correct recipient list using applyOldRecipientLists() is relatively practical now that T4776 is resolved. Note that this will do the opposite of T4776: give you a list of newly added recipients who were not previously part of the recipient list before this round of transactions applied. This may include recipients who have already received messages in the thread, e.g. if they are added, then removed, then added again. Phabricator does not track all users who have ever received a message about an object, as it serves no other goals. This also will not include new recipients who are indirect, e.g., if you have joined a project that is subscribed you the project will be a static recipient; these lists do consider resolution of aggregate recipients into individuals. And if some recipients are mailing lists, all bets are off.

except, perhaps, FreeBSD

For the record, yes, FreeBSD is quite similar and what @chandlerc wrote is very much applicable to our community as well.

Maybe worth a mention -- GHC is another somewhat-similar project and moving to GitLab (e.g., https://mail.haskell.org/pipermail/ghc-devs/2018-October/016425.html). It sounds like GitLab is currently better able to spend resources to support free users with relatively non-mainstream use cases than we are.

As a company, we're self-funded and have generally moved resources away from supporting free users (see T9212, T12681, etc) -- historically, free support has taken up a huge amount of time and effort, been absolutely miserable on a personal level (not you guys), and not clearly driven growth. GitLab may have a better strategy here than we do or may just have more money to burn, but from the GHC thread it sounds like you can ask them for anything and they'll eagerly implement it for free today.

Realistically, our free support is pretty minimal now and not likely to ever return: if you give us a reproducible bug report we'll still often land a fix the same day, but very few support requests are actually "fix this reproducible bug", and the whole gamut of other requests is mostly just a "sorry, we're helping paying customers now". Non-customer users have almost no chance of getting features built upstream nowadays.

While I'm hesitant to recommend you use a competitor, it's possible that GitLab is willing to embrace this class of use case to a far greater degree than we are, and you'd presumably win whether or not their bet that supporting less-popular use cases is a good use of resources pays off or not.

FWIW, I think describing these as "non mainstream" and the entire description of these communities sounds to me ... somewhere between biased and pejorative. Makes it hard for me to keep engaging in the discussion.

Do these represent the dominant or even common users of Phabricator today? Of course not! The whole point is to make Phab easier for communities to adopt. I wouldn't expect *any* existing users of Phabricator to have these workflows heavily ingrained. It was a monumental effort for us to get things good enough for LLVM, and I think LLVM is fairly forward-leaning in this space.

I think by ignoring the pure email workflows, you are missing many opportunities to attract *new* users of the tool.

But, where you spend your resources is of course your call. I just want to advocate for all the development communities that have some developers who would benefit from Phab's interface but not adequately dominant mind share to move the project w/o excellent support for the email workflow they currently rely on.

Anyways, best of luck.

For the record: I'd like us to get away from maintaining our own phab fork, and I agree that means probably migrating to a different system at some point.

Re free support: we don't want this for free :) (it does cost us money as it currently is) IIRC last time I asked whether we could pay to get our features the answer was that you want scale / growth and focus on the main use cases. If that has changed let us know.