The $password variable is not a string anymore in the line after. I guess it's not much more expensive to check if the string $username is nonempty versus comparing to a boolean value

Quick notes:

• For testing code changes, make sure to ./bin/cache purge --caches remarkup
• rPcc44ae32c546: Remove obsolete "setDisableMacros()" on "PhabricatorRemarkupControl" once upon a time existed, could provide a base
  • But how to even realize/pass the info that we are rendering strings in a documentation context?
• PhutilRemarkupRule has a isFlatText($text) method but no setFlatText(true) method
• PhutilRemarkupSimpleTableBlockRule::markupText() is handling our $cell which has the meme/macro name as its only string content
• Afterwards, PhabricatorMemeRemarkupRule:apply() is the next step being called

Not sure if @20after4 has any opinions on this? :) For the records, the code looked like this before this last change: https://we.phorge.it/source/phorge/browse/master/src/applications/config/check/PhabricatorMySQLSetupCheck.php;a41d158490c0cd0a0454653473c39f7ad2b5954f$381-384

My userbase is stellar in relentlessly clicking every button you provide them :P

In T15889#18536, @valerio.bozzolan wrote:

Thanks. What happens in older Apache2 versions?

This happens because if ($continue_on_missing && $error->getIsMissingFieldError()) in https://we.phorge.it/source/phorge/browse/master/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php$1248 is true.

Five months later it seems that we survived. Anything left to do?

We might come from different user journeys.
Your users might be aware what the project means, and that the project (and its workboard) is archived, and they do have an initial reason to look at this archived workboard (e.g. default view is not "open tasks" but "all tasks")?
My users clicked an external link to some project tag in something called Phorge, and that project has been archived in the meantime, and that project is set to show the workboard by default, and that is now just empty columns (as the default view of workboards is "open tasks"), and that is confusing and unhelpful.

Skip assigning variable; direct return

Would anyone be willing to give this another review? :)

Use $caught, not $ex.

• Use $caught, not $ex.
• Include Herald rule monogram in log message for easier finding.

(Hmm should I replace $ex->getMessage() with $caught->getMessage() or not?)

Yep thanks

(Hmm should I replace $ex->getMessage() with $caught->getMessage() or not?)

Correct, the end user will not realize without opening the network tab of their web browser's developer tools.
However, as a downstream admin, I look at statistics/dashboards of HTTP 4xx and HTTP 5xx errors. When the numbers are not very low I'd usually assume something goes wrong and should be investigated. But that is not the case when Phorge throws a "silent" HTTP 400 for no obvious reasons.

Ahh but Transcripts do render an exception when it fails at least. I get a HeraldInvalidConditionException printing Invalid Condition: Condition references a rule which does not exist!, triggered in HeraldAdapter::doesConditionMatch() (only if you reach that condition in the evaluation of course, because https://we.phorge.it/source/phorge/browse/master/src/applications/herald/engine/HeraldEngine.php$437-445 ).
That's called in HeraldEngine in the line $is_match = $adapter->doesConditionMatch (note that class also has a method with the very same name).

For example, some people then may ask "why my default view was changed?" and they will start manually "rollbacking" this to their desired view (e.g. showing closed tasks, from the Workboard, with a filter "Show all tasks").

I personally don't see value in sharing a stacktrace in this case. YMMV :)

Uhmm, transcripts at http://phorge.localhost/herald/transcript/33065/ never ever render a Herald rule condition (disabled or not) within another rule, it seems

I'd be worried about DB performance running such a query as our DB has dozens of millions of files... (This is basically SELECT COUNT(*) FROM file_attachment fa WHERE fa.filePHID NOT IN (SELECT f.phid FROM file); in my understanding?)

Hmm I guess now my patch title is too narrow as it logs any Herald exceptions? :D

Per Valerio's previous comments

Still happens after backing out rP2295bcda14e71948516752f8fbada6601b9f0bde thus not a recent regression.

I guess I just do not enjoy a good number of HTTP 400 errors in our error logs when literally nothing went wrong at all.

The other option would be including "__form__":"1" in that call to avoid the 400.

In D25732#20029, @valerio.bozzolan wrote:

• was this improving the situation? rPf4d9d6920bcd: The feed "created this task" should be the first one seems yes, accordingly to your comments

Applying the debug patch in P45, output when creating a new task shows that Updated (defined in https://we.phorge.it/source/phorge/browse/master/src/applications/transactions/storage/PhabricatorApplicationTransaction.php;877ac8a873f979f418064eb2f36f1148c9838694$1611-1612) wins over Created (defined in https://we.phorge.it/source/phorge/browse/master/src/applications/maniphest/xaction/ManiphestTaskTitleTransaction.php;877ac8a873f979f418064eb2f36f1148c9838694$24) as ActionName:

[0]
PHLOG: 'PATE: TransactionType: core:create' at [/var/www/html/phorge/phorge/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php:1625]
PHLOG: 'PAT: TA Type: core:create' at [/var/www/html/phorge/phorge/src/applications/transactions/storage/PhabricatorApplicationTransaction.php:1601]
PHLOG: 'PATE: ActionName:      Updated' at [/var/www/html/phorge/phorge/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php:1626]
PHLOG: 'PATE: ActionStrength:  140' at [/var/www/html/phorge/phorge/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php:1627]