yuuuuuuup

yuuuup

The culprit is $map[$branch] = $branch_head; in https://we.phorge.it/source/phorge/browse/master/src/applications/diffusion/data/DiffusionGitBranch.php$105.
Before $branch was a string, afterwards it is an integer.

Patch approved in 123123121231323μ asd

Yup and thanks that somebody already put "PHID" is the inline description.

Looks good to me, thanks!

Looks good, yeah.

In D25935#25149, @Cigaryno wrote:

Why would a cancel URI be needed?

@avivey does this look good to you?

In D25926#25147, @Cigaryno wrote:

But so far this is nothing meant to be hidden from users who can't edit the repo.

In D25935#25135, @aklapper wrote:

After these steps I get Unhandled Exception ("Exception"): This transaction group requires MFA to apply, but the Editor was not configured with a Cancel URI. This workflow can not perform an MFA check.

Why would a cancel URI be needed? Do you know a Cancel URI for an app with something that prompts for MFA (ie. exposing Passphrases, empowering users, signing comments with MFA, managing your VCS password and SSH keys)

In D25935#25144, @Cigaryno wrote:

In D25935#25135, @aklapper wrote:

Which "an application" exactly?

Any application were canUninstall is not set to false (thus not a required application).

That's what I tested (as the Files application can be uninstalled). Which exact application(s) did you test?
I'm surprised that you did not run into the same problem as I did described in my last comment...maybe it's related to not being an admin?

In D25926#25145, @aklapper wrote:

Socially I remain unconvinced about use cases. Implications are for example exposing hidden (or internal?) URIs under URIs or "Working Copy Status" stuff under Basics to the public. I just so far do not think it's a good idea.

Tested this locally; technically it looks correct to me.

In D25935#25135, @aklapper wrote:

Which "an application" exactly?

Any application were canUninstall is not set to false (thus not a required application).

As which type of user?

A user with the Can Configure Application capability (by default admins).

Fix typos reported by @aklapper.

In D25936#25132, @aklapper wrote:

@Cigaryno: Thanks! Could you elaborate why the change in .arcconfig is needed?

Clear Test Plans with URIs are welcome - the less others need to think "how/where to do that" the easier gets testing.

@Cigaryno: Thanks! Could you elaborate why the change in .arcconfig is needed?

So, this turns out to be a lot harder than I thought.

Should be fine after these two changes :)

There is a downstream patch "Add HTTP Body option in Harbormaster HTTP Build step" in https://github.com/mozilla-conduit/phabricator/commit/9bb9a12079b381ce14af97a8b2c511ae71be389c which might be related and could be lifted?

I think it's relevant to have a status site (likely at status.phorge.it). Uptimerobot sounds like a good option and is used by translatewiki.net.
One thing that's optional to have (likely not possible with Uptimerobot) is a Daemon status checker, as @avivey pointed out when they forgot to start the daemons at some point. For this, a custom status checker that calls a daemon that's not for making edits or changes can be used.

I don't know why, but O1 has to be manually added as a reviewer as it does not own R10.

Thank you both for the conversation here and further thanks @Cigaryno for the patch! :)

I don't know if what I done on D25935 is correct, but as usual, Request Changes if I did it wrong.
I relied on the code on PhabricatorUserEmpowerTransaction to try to make PhabricatorApplicationUninstallTransaction require MFA if enabled.

Make lint happy

Mention closed-source apps in addition to open-source apps per @aklapper

Per @aklapper, it's best to show both closed-source and open-source TOTP apps.

In T16018#21478, @Cigaryno wrote:

In T16018#21476, @aklapper wrote:

I'd personally not remove common proprietary software options (as it makes life of users potentially harder if they already have such an app installed) but list FOSS options first.

Some FoSS devs may not be familiar at all with open-source TOTP apps. I personally use Google Authenticator so I agree with you and also, I have my TOTP content on WinAuth too, which is unmaintained however I am not ready to switch TOTP app on my Windows PC (my revs from now on are created from an Ubuntu VM due to the arc troubles I am having on Windows).

In T16018#21476, @aklapper wrote:

I'd personally not remove common proprietary software options (as it makes life of users potentially harder if they already have such an app installed) but list FOSS options first.

In D25934#25089, @aklapper wrote:

I'd prefer not to remove common proprietary software options but list FOSS options first.

I'd personally not remove common proprietary software options (as it makes life of users potentially harder if they already have such an app installed) but list FOSS options first.

I'd prefer not to remove common proprietary software options but list FOSS options first.

I will submit a patch shortly.

In D25926#25064, @aklapper wrote:

What is there to "further review"? It's two lines...

What is there to "further review"? It's two lines...

Can this be further reviewed?

Thanks for the hint.

Well done \o/

Remove duplicated assignments to $stderr

Add link to T16023

Fix lint message

Update to detect stderr messages

As said in https://we.phorge.it/D25931#25023 I think this is a sub-task of T15243. And in this specific case, very probably we don't want to log anything. So your report could be considered a regression caused by T15243.

Thanks my friend. I 100% understand the problem and I'm happy you shared the stack trace in the task and I'm happy about this test plan that helped me to reproduce,

arc diff