- User Since
- Jul 3 2012, 3:29 PM (551 w, 5 d)
Jan 3 2019
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.
Oct 6 2016
@epriestley - makes sense; I've been approached by some folks about our setup because they wanted kernel devs to get an actual code review system, who apparently have a similar mindset to compiler developers, but that's probably a long shot anyway :)
Note that this feature (text mode unified diff) is crucial for our deployment (requirement for text-only mails to contain reasonable diff snippets).
Apr 13 2016
Jay: ok then, while I'll not argue about whether breaking is correct or not, gmail does break citations correctly.
That is, if I open an email and write (break before "unrelated newline"):
Aug 10 2015
@adrelanos: yes, that's what this ticket is about.
Jul 3 2015
Jul 2 2015
Jul 1 2015
Awesome docs, thanks!
Any chance to get this in? Perhaps under and option? Eitan, how are you guys in FreeBSD land handling this?
T8724 has an implementation idea on how to fix this. Would there be interest in a full patch?
Jun 30 2015
Ok, just tested replying to the email inline, and I get the following error:
"""Your email to Phabricator was not processed, because an error occurred while
trying to handle it:
Jun 3 2015
@epriestly - thx for the heads up. /me fears the next integrate ;)
May 28 2015
Jan 20 2015
Aug 20 2014
Awesome. So what about the rest? I'm happy with keeping this in our branch (I think it's never led to a merge problem), but I also think it's super useful to at least have documented somewhere if we don't want to put this into the main branch (it took me a day to figure out what the right solution is (after talking to some of my colleagues), so if somebody else hits this they'll probably also be wasting a lot of time on it; figuring out why mail is rejected is about the least fun thing one can work on).
Aug 15 2014
- Address review comments
- Merge to head
Aug 13 2014
Regarding your effort to maintain this, a few thoughts:
- Thank you so much for doing it; no matter whether you take this in the end, I really appreciate your feedback and time; I understand the trade-off between features and maintenance headaches, so I fully appreciate if we have to keep our own fork.
- That said, keeping our own fork often feels very very frustrating; I know that's not your problem, but I think that 99.9% of the time I've spent countless hours in syncing and resolving merge conflicts would have cost no time if the person writing the feature (-> you ;) would have changed the code with the refactoring (I love the refactorings, just not the merging ...); that would save so much of my time that you dropping me an email whenever there is a problem with this code so I can resolve this would be immediately prioritized for me, based on that I would lose so much if I have to keep our own fork
- <<</me opens the bag and pulls out a carrot; the carrot looks somewhat translucent, so you can't really be sure whether it exists>>>: we would very much like not having to maintain our own instance, and we'd like to become paying customers (the countless hours I throw at maintaining our own GCE instance with this stuff are beyond reasonable); not sure whether you still aim at providing such a service though...
- I think the feature might not be used very much until now because it's not feature complete yet; I think the comments in the diff actually make the whole email diff thingy much more useful, and if we add parsing in-line replies and adding them to comment hunks as replies, that would make it even more useful (it would mean I could just do reviews completely by email if I have enough context); but of course I would say that, so probably not a very convincing argument on its own :P
Address review comments.
Aug 12 2014
Aug 5 2014
I think this broke my install - told me something about running "arc liberate", but running that didn't fix the issue.
Aug 4 2014
Actually works quite nice (I have worded the text to suggest to create a new revision). Now I have to find out where I can make the warning field screaming red ;)
Ah, this works, but only after-the-fact. The problem is when we add a mailing list to cc' after the fact, the mailing list doesn't get a mail that looks like the "request for review" mail, so people on the list are missing context (which is another thing we want to fix but haven't gotten around to).
Another reason is that the email parsing doesn't work for inline replies, and we basically always do inline replies.
So if we replied only to phab, we'd basically restrict how users are able to write emails, which they will simply not accept.
Ah, sorry, just read the commit comment...
I just wanted to try it - any reason why this was reverted again?
Jul 11 2014
The problem is that many people only use the mailing list and don't do
reviews via phab themselves (but still want to be part of other people's
review threads). I think having different flows for phab vs non phab is not
going to fly for us.
btrahan added a subscriber: btrahan.
btrahan added a comment.
Jun 27 2014
That definitely sounds like a very good first step. We would reevaluate how often unintentional private reviews happen afterwards. Thanks!
Jun 10 2014
Jun 5 2014
I tried to reply this via email, but email delivery failed
Delivery to the following recipient failed permanently:
May 31 2014
May 26 2014
We have a setup where:
- all phab reviews also go to a mailing list as well as to the specified reviewers (if any)
- people reply on the mailing list to phab review emails via reply-all
- we do not want phab to be in the critical path for reviews, thus a reply-on-to-phab setup is not an option
I think getting to an 95% solution shouldn't be too hard (mainly effort, which is why I haven't done it myself yet), and just including most of the email for the 5% where we don't get it right seems better than not having the comments in the phab thread at all. Then I'd guess it's 1. wait for bug filed, 2. add regexp, 3. redo from start.
Apr 30 2014
Jan 7 2014
This is getting more and more of an issue - now the reverse case happened - we had a user set up a phabricator user name that (by chance) matched the subversion user name of a different user.
Dec 11 2013
The message exists, and looks exactly normal:
Message ID Hash: BvDfx.DmqDTw
Ah, I think with one of the recent integrates I'm back at the point where I get an error due to an empty message body. I remember inserting code to switch that error off, but I'm unsure of where I had to pull the lever...
Btw, this is becoming more and more annoying - if there are any tips on how I can debug this, that would be highly appreciated :) (the problem might well be related to my local changes...)
Dec 6 2013
Nov 20 2013
Nov 18 2013
Ok, that worked... what's the best way to put the email up without it
getting indexed by search engines (the content is all public, as it goes to
a public mailing list anyway, but I don't want the full mail addresses to
show up indexed in search engines).
I by now had multiple people reporting this problem. Do you still have
admin access to our phab instance (and can you get to the stuff there?)
Otherwise I'll dig for the original...
Nov 15 2013
Oct 11 2013
Ah, sorry - I overlooked the notice because it was in the middle of the comments section (which was already large) and the file section.