Phabricator not getting all comments sent by email?

Does anyone know why phab sometimes misses replies sent by email? Usually they make it through, but sometimes not.

For example, a recent email from Rafael (which I got through the llvm-commits list) seems to be addressed to the proper thing @reviews.llvm.org, yet seems to have never made it onto the website.

Not having not seen that response (because I was looking on the website), I ended up writing effectively the same thing a second time. Oops.

I think I've noticed it dropping e-mails that start with quotes
before, and certainly ignoring everything below the first quote line.
It seems to assume everyone will be top-posting.

Tim.

GAH! Sigh… :frowning:

That is indeed what it does. See stripQuotedText in src/applications/metamta/parser/PhabricatorMetaMTAEmailBodyParser.php if anyone’s interested in hacking on php code, it looks like it might be fairly straightforward to have it strip only the lines starting with “>” after the “On … wrote:” line.

James Y Knight via llvm-dev <llvm-dev@lists.llvm.org> writes:

GAH! Sigh.... :frowning:

That is indeed what it does. See stripQuotedText
in src/applications/metamta/parser/PhabricatorMetaMTAEmailBodyParser.php if
anyone's interested in hacking on php code, it looks like it might be
fairly straightforward to have it strip only the lines starting with ">"
after the "On ... wrote:" line.

Wouldn't that be confusing? Such comments tend to refer to the quoted
text just before the comment. We'd probably want to keep some arbitrary
number of the quoted lines before each comment or something.

Note also that if we fix that we should probably also teach phab not to
re-send comments to the list when the list and phab were both on cc.
AFAICT any top posted comments on phab reviews show up on the list twice
currently, once from the author and once from phab.

James Y Knight via llvm-dev <llvm-dev@lists.llvm.org> writes:

GAH! Sigh… :frowning:

That is indeed what it does. See stripQuotedText
in src/applications/metamta/parser/PhabricatorMetaMTAEmailBodyParser.php if
anyone’s interested in hacking on php code, it looks like it might be
fairly straightforward to have it strip only the lines starting with “>”
after the “On … wrote:” line.

Wouldn’t that be confusing? Such comments tend to refer to the quoted
text just before the comment. We’d probably want to keep some arbitrary
number of the quoted lines before each comment or something.

Note also that if we fix that we should probably also teach phab not to
re-send comments to the list when the list and phab were both on cc.
AFAICT any top posted comments on phab reviews show up on the list twice
currently, once from the author and once from phab.

Maybe file both of these in the Phab bug component?

Doesn't seem to be a trivial thing to do:

https://secure.phabricator.com/T7358

https://secure.phabricator.com/T5181

cheers,
--renato

Just ran into another thread where phabricator is seemingly ignoring replies. This one seems to be a different issue.

In the thread “[PATCH] D20337: [MC] Support symbolic expressions in assembly directives”, Phabricator seems to have completely ignored all of the replies starting with my (emailed) reply earlier today: “The .s does have a way to carry the location.”.

Except for the latest “test” email, where I moved the phab address from the “Cc” line to the “To” line. It seems to have gotten that. So maybe it ignores emails unless it sees itself in “To:”???

Would it make sense to officially have phabricator ignore all replies to the email thread, and instead require that all comments are done through phabricator itself?

-Krzysztof

Would it make sense to officially have phabricator ignore all replies to
the email thread, and instead require that all comments are done through
phabricator itself?

The official stance is that people should be able to interact with reviews
from the mailing list without ever registering for/using Phab, so that
probably isn't acceptable.

That would be even worse, IMO.

Joerg

Perhaps it should only strip quoted text;

  • if it appears in a continuous block
  • there is only new content above or below
  • the content “matches” an earlier comment, ignoring whitespace etc.

Or just preserve it all, but provide a way to toggle visibility when displayed in the web front end, ala gmail.