In T15815#17273, @valerio.bozzolan wrote:

I also don't think Phabricator ever documented how to change language in command line Arcanist. If ever possible, btw 🤔

fix abstract function visibility, add more PHPDoc

I'm purely setting "Request Changes" to reflect that IMHO this should not get merged until there is an approach to fix T15579 - otherwise in some corner cases this proposed change would create broken URIs.

I'd prefer not to expose this as an always-visible button simply because it takes screen estate on smaller screens.
Maybe an entry Copy Revision Permalink between two separators in the expandable "View Options" dropdown is an option?

For the record: If I replace the entire file with just:

<?php

(that was me, I didn't have git author configured properly from on the system I landed from)

(that was me, I didn't have git author configured properly from on the system I landed from)

arc unit --everything passed. But I don't think this file is covered by tests at all,

Setting high priority since I'm scared :D Feel free to re-tune

I was not aware that we were executing critical business code inside __destruct() automagic calls. Wow!

Had this issue occur elsewhere, so put a general 'fix' for it:

In T15815#17273, @valerio.bozzolan wrote:

Thanks. Completely agree.

I also don't think Phabricator ever documented how to change language in command line Arcanist. If ever possible, btw 🤔

In D25626#17531, @aklapper wrote:

This seems to also fix http://phorge.localhost/feed/ so it does not only show me about setting high priority on a new task, but instead shows that the task was created? That would be a nice side effect too IMO.

Maybe relevant, from my Postfix logs:

I can confirm this works as expected for the task view itself in e.g. http://phorge.localhost/T1168 . This seems to also fix http://phorge.localhost/feed/ so it does not only show me about setting high priority on a new task, but instead shows that the task was created? That would be a nice side effect too IMO.

Tested this patch on http://phorge.localhost/feed/transactions/ and in the feed at the bottom of http://phorge.localhost/W20 itself.
Also tested changing panel visibility from Public to Admin only and accessing these URIs as an average user.
All working as expected, no explosions.

Thanks. Completely agree.

In T15815#17250, @valerio.bozzolan wrote:

being able to have people running unit tests and see the results in their language was a feature

In T15096#12185, @valerio.bozzolan wrote:

Another problem dramatically frequent for newcomers (at least in my office).

It's relatively too much easy to start with a similar commit message:
...

(run the full tests, and then count it as my + O1 Accept).

I think the costs of the extra translations are low, but the cost of "the tests are not being translated for developers" are even lower.

I'd be worried that this might break some tests that do relate to translation/extraction, but running arc unit --everything should solve that.

Thanks. To reviewers: maybe relevant, maybe not:

I 100% see the point of avoiding to "waste translators' time", but I also see that completely removing tests is a big choice for upstream: being able to have people running unit tests and see the results in their language was a feature, so we also "waste a feature".

I tested this one-liner as part of the downstream conversation that pppery and I had in https://phabricator.wikimedia.org/T363364, still I would ideally prefer another upstream opinion before accepting this revision as O1.

Re-diff with linting and tests working hopefully

fix "abstract" function name

Thanks. Confirmed reading this:

Nice catch :) Can reproduce. Tested.

cover all known usages, but set as stub

At least, please go to GitLab, instead of GitHub, so at least you use Free Software :)

Something I stumbled upon few months ago (via forgejo): agit-flow[^1]. Maybe worth sharing here -- for teh record? It does have some CLI client (git-repo) but it's optional and push via git push origin HEAD:refs/for/<target-branch>/<session> would trigger the Code Review process, without gerrit's Change-Id hack.

Just some (probably final) thoughts on arcanist and phabricator/phorge. After quite a bit of discussion internally, we are moving on to GitHub with our repositories (which were previously stored in another Git server - not Phabricator). But that move also spells the end for our/my 10+ year long use of Phabricator, because when we move to GitHub, we'll also be ditching Phabricator for code reviews.

lol

After upgrading PHP from 8.1.22 to 8.1.27, problem fixed. Thank you again, bozz.

Thank you, Bozz.

Thanks again for this patch that highlighted the valley and the mountain in a very clear way.

In D25608#17388, @avivey wrote:

This is probably the right fix (or maybe moving shouldAllowPublic() to the base PhabricatorFactController).

This is probably the right fix (or maybe moving shouldAllowPublic() to the base PhabricatorFactController).