I forgot to accept but please consider the little cute small comment to add the corner case for the background ajax request, that I suspect it may work too and I hope with less unexpected situations. There was probably a reason for that isFormPost(). For example see isContinueRequest() that is even stricter in a way. Thanks :)

I like it :) Thanks! It works on my machine following the test plan.

(we can probably keep this ticket open, so that we have the 2nd part on the backlog. I'm pretty sure we want it to happen "eventually".)

Feel free to show something early :) That would attract more attention than the Discussion Needed tag I bet

Wrote the code for the first phase :p

Sounds good to me :p

Thank you!

Let’s do it

And another typo

If there are no objections I would be happy to accept the diff. @speck are your concerns addressed or should we continue discussion / consider other options?

fixed capability

In T15965#20144, @valerio.bozzolan wrote:

What is changing is, that unverified email will not match your unverified email as default, so that should need these 2 clicks manual configs (or, find a way to verify the email)

Yep, manually setting your unverified (and not verifiable) email would still be possible 👍 just two clicks are needed from this kind of pages:

Another edge case: Most of my contributions to Phorge happened as part of my work for Wikimedia. Those commits are under an email address that I no longer have access to, since I am no longer employed at the Wikimedia Foundation.

(I cannot edit this task lol - I would like to add Spam mitigation tag to keep an additional eye on these nice things)

In D25775#21206, @valerio.bozzolan wrote:

More understanding on the root cause is needed. Probably the root cause is "just" that getOldValue() returns an empty string. In that case we should probably at least understand what object is that (sub-class of PhabricatorTransactionRemarkupChange?) and we probably we need something like a generateOldValue() or something similar.

After installing subversion and setting LC_ALL instead of LANG I can finally reproduce on a Fedora 40 system:

Oh true, got it. Have to replace PhabricatorPolicyCapability::POLICY_ADMIN, with 'capability' => PhabricatorPolicies::POLICY_ADMIN, here

In D25850#22726, @valerio.bozzolan wrote:

What happens to already-existing URLs? Maybe nice to mention in the test plan

We can also ship this feature in two phases, so, first, adding the option files.maximum-file-size, and then the second one when it's ready or requested lol

Yeah, I agree, though I would then only work on implementing files.maximum-file-size because we don't really care that much about adding exceptions to the rule (as far as I know lol)

last change promise lol

arc unit

harden

\o/

also tried to fix PhutilRemarkupEngineTestCase

but fails in link-edge-cases.txt now (thus it's likely not complete):

Double slam-accept

Uh, that would be so good. So you can say "When the moon is full".

Sounds reasonable.

Take for example this commit that has a default identity:

"Steal credit" might actually lead to a real issue: If a new user can get themselves identified as an old, trusted, user based on commit history, their changes might not be checked as rigorously by the rest of the team - similar to the XZ Utils backdoor issue, only faster.

Limitation: to steal a commit identity, it must be the default. Sorry I forgot to say.

In T15965#20052, @speck wrote:

What can a malicious user accomplish by claiming unverified email for commits?

I like your option names. I like to specify PHIDs and not numeric IDs so it's more portable against import/exports 👍 Let's add Discussion Needed to attract some +1 or nice suggestions.

What happens to already-existing URLs? Maybe nice to mention in the test plan