Good question. The documentation is not "super-opinionated" about this - https://we.phorge.it/book/contrib/article/unit_tests/

Open question is whether to replace loadAllWhere with loadOneWhere under the assumption that there are always either zero or one results.
That would definitely require adding null check though.

In D25807#21516, @valerio.bozzolan wrote:

P.S. try applying D25809 to check if the red unit test becomes green

P.S. try applying D25809 to check if the red unit test becomes green

cc: @avivey

@avivey I added task T15928 and will get started with that.

Thanks again. Let's see if we receive some feedback from users in master branch

I had to rebase and rebuild resources/celerity/map.php to fix a merge conflict. I hope that worked.

Oh, I have to land it… Let's see if I still remember how.

Thanks. Premising that I also don't like projects with "automagic getters" so I agree with you; here how every getter works in Phabricator/Phorge:

Also related: T15025

Change offset to optional as requested by speck

As usual I'm confused about the PHP magic which does not require defining the method PhabricatorRepositoryCommit::getEpoch() in its class or parent classes, but I assume you tested locally that your patch works, plus PhabricatorRepositoryCommit does have some calls already to $this->getEpoch(), so I'll accept - worst case would be the very same behavior as now.

@valerio.bozzolan Thanks, this unblocks a few more things on my list. I admit that I have not looked into the unit test failure here and I assume you understand it and that it is fine to merge this patch. :)

FYI T15927: Fix Arcanist unit test ArcanistBundleTestCase->testNonlocalTrailingNewline() on non-English terminal

Width overflow fixed

Remove unecessary comment