git rebase master

git rebase master

OK, tested by 3 hackers, no git regressions, more SVN stability, more inline documentation, more abstraction, very Phorgi, already tracking follow-ups, yuuup

In T15957#22436, @keithzg wrote:

Testing this a bit myself again, I discovered that actually this problem affects both Git and SVN

Testing this a bit myself again, I discovered that actually this problem affects both Git and SVN, it just wasn't being noticed in my SVN testing because it doesn't go wrong if run from any subfolder. So for example in a checkout of rARC, running arc browse . in the root directory will pop one into the bogus path of, say, https://we.phorge.it/source/arcanist/browse/master/home/keithzg/Code/git/arcanist, but if I'm in my local ~/Code/git/arcanist/src/ then arc browse . pops open https://we.phorge.it/source/arcanist/browse/master/src as intended.

The only remaining issue I could find in retesting was that, when run on the root of a repo, apparently SVN exhibits the same behaviour as Git does in T15957; and furthermore in my testing actually Git also doesn't have that problem if arc browse . is run in a subfolder rather than the root of a repo. (It's just so much more common to be in the root of a Git repo, and so much less common to be in the true root of an SVN repo, that we didn't notice!)

Thanks, @valerio.bozzolan! :D

Thanks @Chlod for this bug report and please also enjoy T16057 and the new documentation table (D25988) that is provided to you for free :3

That was my patch initially but it felt like too much duplicated code... I don't have a strong opinion though; I'm also fine with that other approach. :)

I wonder if we should just extend PhabricatorProjectTriggerRule. So, leaving PhabricatorProjectTriggerManiphestOwnerRule as-is 🤔 any opinion about that?

What's the expectation for classes which have a child class extending them? Marking them as abstract though they are really not abstract at all? protected class? Uhm.

Better now, thanks again @avivey

Tested right now. Unexpected pleasure! thanks! :3 :3

Uhm this is a damn rabbit hole.

@valerio.bozzolan core/remarkup.css has five more unused selectors so I'd put them all together into one patch.

Maybe also we can drop something from ./webroot/rsrc/css/core/remarkup.css - since it seems to contain .aphront-panel-preview ( .aphront-panel-preview .phabricator-remarkup-mention-unknown)

another round of bin/celerity map

I guess this welcomes another review / testing. Works fine here.™

Also update comment

Thanks. I pinpoint also here your super-relevant extra details: https://we.phorge.it/D25887#26924

In D25887#26923, @valerio.bozzolan wrote:

Just for my curiosity, can you please also follow the tip from the screenshot (before the patch), so, using --trace?

Trying the ManiphestTransactionEditor leads to a dead end. Added personal notes about it...

Just for my curiosity, can you please also follow the tip from the screenshot (before the patch), so, using --strace?

In D25887#26917, @avivey wrote:

I'd still not expect it to actually break anything - the docs say this const is "deprecated" not "removed", so I'd expect a warning and everything to keep working.

In D25886#24266, @valerio.bozzolan wrote:

Since using E_STRICT is deprecated, I thought the simple fact of using it from whatever Arcanist CLI