@eadler, please don't "Accept" patches unless you are confident that the code is correct and that you have the experience and context to make that call. This usually means that you wrote the affected code or have significant familiarity with similar code arising from a history of work in the codebase.

In particular, if you "Accept" a patch, you should assume it may land immediately and be willing and able to help fix problems which are caused by problems with the code. "Accepting" code implies taking responsibility for its correctness.

Although this particular change really is as straightforward as it looks and you might reasonably be able to help resolve problems with it, you've accepted code in the past which I suspect is currently beyond your ability to assist in resolving problems with (like D13120). There's also a practical limit to your ability to take responsibility for fixing things because you don't currently have commit access.

This causes problems because it pulls changes out of, e.g., my "Needs Review" queue, and marks them as "Accepted" in views like arc list. For example, I landed D13545 on your authority by mistake (I believed @btrahan had reviewed it). This change did ultimately cause problems (fixed in rP7c6320d2). There was probably no actual difference in outcomes here between your review and a @btrahan review (it's certainly possible that @btrahan would also have missed the problem, and the issue was minor and I was available and able to resolve it fairly promptly), I just mean to illustrate that this behavior does concretely erode safety checks in the process, which assumes that "Accepted" means there's a second party who has bought into taking responsibility for the change.

If you just want to express that you like a change, consider awarding a token or leaving a comment.

If you want to keep track of changes for your own purposes, you can flag them.

It's also completely OK to continue reviewing changes and leaving comments or suggestions, but please don't "Accept" them unless you're confident about taking responsibility for the changes.

To some degree, there are technical improvements we could make here, but the current system -- which includes some "soft" cultural rules like this one -- works well in most cases, and better in some cases than encoding all of these rules in hard technical rules would (for example, we the "soft" rules can more easily be bent for typo fixes and minor changes where no real responsibility is required).

This is probably at least part of the feed/mail problem:

[2015-07-27 07:42:06] EXCEPTION: (PhutilProxyException) Error while executing Task ID 239613. {>} (Exception) Bad getter call: getName at [<phabricator>/src/infrastructure/storage/lisk/LiskDAO.php:1700]
arcanist(head=master, ref.master=5fcf7b5a3b9e), instances(head=stable, ref.master=418ae876df49, ref.stable=418ae876df49), ledger(head=master, ref.master=4da4a24b8779), libcore(), phabricator(head=master, ref.master=cb1f4ea72197, custom=18), phutil(head=master, ref.master=aa6cd8f7e5e5), services(head=master, ref.master=576ad99b5406)
  #0 <#2> LiskDAO::call(string, array) called at [<phabricator>/src/infrastructure/storage/lisk/LiskDAO.php:1678]
  #1 <#2> PhabricatorCountdown::getName() called at [<phabricator>/src/applications/countdown/editor/PhabricatorCountdownEditor.php:163]
  #2 <#2> PhabricatorCountdownEditor::buildMailTemplate(PhabricatorCountdown) called at [<phabricator>/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php:2323]
  #3 <#2> PhabricatorApplicationTransactionEditor::buildMailForTarget(PhabricatorCountdown, array, PhabricatorMailTarget) called at [<phabricator>/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php:2283]
  #4 <#2> PhabricatorApplicationTransactionEditor::buildMail(PhabricatorCountdown, array) called at [<phabricator>/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php:1046]
  #5 <#2> PhabricatorApplicationTransactionEditor::publishTransactions(PhabricatorCountdown, array) called at [<phabricator>/src/applications/transactions/worker/PhabricatorApplicationTransactionPublishWorker.php:21]
  #6 <#2> PhabricatorApplicationTransactionPublishWorker::doWork() called at [<phabricator>/src/infrastructure/daemon/workers/PhabricatorWorker.php:91]
  #7 <#2> PhabricatorWorker::executeTask() called at [<phabricator>/src/infrastructure/daemon/workers/storage/PhabricatorWorkerActiveTask.php:162]
  #8 <#2> PhabricatorWorkerActiveTask::executeTask() called at [<phabricator>/src/infrastructure/daemon/workers/PhabricatorTaskmasterDaemon.php:22]
  #9 PhabricatorTaskmasterDaemon::run() called at [<phutil>/src/daemon/PhutilDaemon.php:183]
  #10 PhutilDaemon::execute() called at [<phutil>/scripts/daemon/exec/exec_daemon.php:125]

Specifically PhabricatorCountdownEditor->buildMailTemplate() calls getName(), but should call getTitle() (Countdowns have a title, not a name).

I found this by:

• Stopping the daemons.
• Creating a countdown.
• Running bin/phd debug taskmaster and looking for error messages in the output.

awww yiss, I wondered if it was in the workers somewhere. I kept digging and then I got lost in there. I didn't get anything in the normal logs.

thank you thank you thank you