Double-accept

In D25823#22340, @keithzg wrote:

... Conversely, arc browse . works for what I expected a pathless invocation to do, and funny enough that doesn't work on Git repos! So we have feature parity ;)

In D25823#22324, @valerio.bozzolan wrote:

Hi @keithzg (and any SVN hacker) do you like the new added Subversion support?

Yep. I'm up-to-date with master as of filing the issue^^.
This textbox isn't a Remarkup field (it's actually a ConpherenceDurableColumnView, which doesn't even have a metadata field in the first place)
In normal Conpherence views, the "Upload file" button works just fine. (except as mentioned in Z1, where reloading the page erases the metadata field)

Thanks. Can you reproduce with "Upload file" button too? If yes, maybe sub-task of: T15106: "Upload file" in remarkup text fields should attach by default

@avivey Thanks for the tip about arguments allow-list in getArguments() https://we.phorge.it/source/arcanist/browse/master/src/workflow/ArcanistLintWorkflow.php$67 but I was not able to use that since here it's using getWorkflowArguments() 🤔 So I've just added a warning. It works lol. Check "BRANCH OPTION NOT AVAILABLE"

found a corner case, fixed

I think that, after the hidden comment is loaded in its bulk, "some more comments" are loaded as context for the hidden comment 🤔 So I guess that extra loading is probably a feature and not a bug.

Wikimedia Foundation has this in production since years. I think it doesn't hurt.

Hi @keithzg (and any SVN hacker) do you like the new added Subversion support?

Probably not enough consensus for a mass edit. Best we can do is case-by-case fixes reading warn reports.

Any other test plan suggested? Current ones: delete simple milestone, delete project with milestone, delete a project with sub-projects, delete a sub-project, ecc.

One weird observation:

helping reviewers, adding comment on future export possibility

fix lint, happy review

it works!

This very patch got deployed on our rather active downstream instance 10 days ago. I have not heard complaints and I've seen tasks and task comments (replies) created via email, so I'm now pretty confident that it does what it should. Thus I'm going to merge it to reduce technical debt a bit more in Phorge.

add extra escape

OMG IT WORKS

Thanks. Seems good to me since trim($new) does not receive null so cannot create PHP 8.1 problems, since we already execute strlen($new) and mb_strlen($new) and that was not reporting errors in your updated install

@avivey: Would you like to give this another review, once you find some spare time? TIA!

Restore an empty line

I see the API maniphest.createtask is frozen, maybe this can also create tasks: https://we.phorge.it/conduit/method/maniphest.edit/

• APC: Change the setKeys $ttl parameter default to 0

This exception happens once $rule in the loop foreach ($this->getMarkupRules() as $rule) in PhutilRemarkupBlockRule::applyRules($text) becomes ProjectRemarkupRule. That's where it blows up.

Think I may have found what is causing this. We had a few repositories that auto download/sync from github. The user's key who connects to github was missing so the downloads were giving errors. I think this triggers a memory leak for the php process. Now that I've fixed the key issue no errors now appear and phorge looks to be happier, i.e. memory usage looks minimal all the time (i.e. php-fpm processes using ~100MBytes).
I'm going to keep monitoring for a few weeks.
Here's an example from the log: