@Cigaryno: Please provide references when you quote something from somewhere so others don't have to guess where to find things. Thanks!

In D25859#24307, @mainframe98 wrote:

Shall I just land this then?

This doesn't seem correct. renderObject doesn't take any arguments. I suspect the intention was renderHandle, as that method takes a PHID, which the metadata key mentions.

In D25868#23446, @slip wrote:

I found this, which suggests that this shouldn't be used for headers. https://secure.phabricator.com/D21862

Followed test plan, confirmed findings, accepted revision.

In D25859#24228, @aklapper wrote:

Yes, I'd like to see this merged, [...]

Use !empty instead of isset to match previous type juggling behaviour

@mainframe98's patch for fixing Packages makes me think about the current contribution policy for prototype applications. It currently states:

With rare exceptions, we do not accept patches for prototype applications for the same reasons that we don't accept feature requests or bug reports.

Maybe this is one obsolete Phabricator-specific contributing policy?

Thanks

Probably no need for a warning. Thanks

rebase

Since using E_STRICT is deprecated, I thought the simple fact of using it from whatever Arcanist CLI (e.g. arc anoid) was triggering this new warning

Thanks, I can confirm that this works as expected. :)
I agree biggest question is where to put this. In my opinion the UX is already slightly inconsistent (given their are also concepts like "Related tasks" in Pholio or "Referenced Files" in tasks).
In Differential it feels a bit like a stretch to put this into a box called Revision Contents.
In Maniphest the same "Mentions" tab is in the Related Objects box; there is nothing similar in Differential which already has numerous boxes (also Details and Diff Detail but none of those have tabs).

Superseded by D25861

Because the email address is set no noreply@secure.phorge.dev, it does not seem to be possible with standard emailers to email replies. Blessed Roots may try to add Application Emails to Maniphest and Differential.

close enough for my taste; Just add . at the end of each sentence.

Done. Thank you for your tips!

@xtex: Hi, would you like to arc land your patch? Or do you need any help? Thanks!

@xtex: Hi, would you like to arc land your patch? Or do you need any help? Thanks!

In D25886#23757, @valerio.bozzolan wrote:

Finally something that can be tested using arc anoid - I guess?

rebase? please?

./bin/celerity map for real, sigh

rebase to drop unrelated wrong celerity map change

Yes, I'd like to see this merged, as it fixes going to my local user profile currently only showing

PhutilAggregateException
Encountered a processing exception, then another exception when trying to build a response for the first exception.
- RuntimeException: Undefined array key "totalAsCurrency"
- RuntimeException: Undefined array key "totalAsCurrency"

on PHP 8.3.17 after performing the steps in the Test Plan above.

Beforehand,

• Created a publisher at http://phorge.localhost/packages/publisher/edit/form/default/ but afterwards, http://phorge.localhost/packages/package/publisher/edit/1/ is a 404.
• Created a package at http://phorge.localhost/packages/package/edit/form/default/ but afterwards, http://phorge.localhost/packages/package/package/edit/1/ is a 404.
• Created a version at http://phorge.localhost/packages/version/edit/form/default/ but afterwards, http://phorge.localhost/packages/package/version/edit/1/ is a 404.
• Going to http://phorge.localhost/packages/ is a 404, had to go to http://phorge.localhost/packages/package/ instead.

After applying this patch locally, all these four URIs work as expected.

setControlInstructions() is the right approach, after looking at other usage of it.
Tested the patch locally and works as expected.
I'd probably rephrase to "Callsign is the monogram rXXXX for this repository. It cannot contain spaces." and "Short Name is part of the URI paths for this repository. It cannot contain spaces." (we don't seem to use "repo" but only "repository" across the codebase, and use full stops), and drop the Placeholders, and rename the patch to "Show descriptions for "Callsign" and "Short Name" Repository form fields" (or such).
Apart from all that my nitpicking, good to go. Thanks for the patch!

Rename source files from Tasks to TasksAssigned for consistency; update two more exposed strings

What do you think about @mturdus?

I have also noticed this issue when submitting updates to a revision -

I found a method called setControlInstructions that adds helpful text above the field

• Refactored to use the setControlInstructions method on the control

How would this work if the task has more than one revision associated with it?

There is one way to sort of do this already, by using milestones for your columns, then you can use herald 'add projects' rules. This only works by sort of abusing the purpose of milestones to take advantage of the fact that milestone columns are mutually exclusive. Adding one milestone removes the other milestones from the same parent project.

I agree that the hover behavior could be improved. Showing only after a delay could help here, though that would make it slightly worse for the times you actually want to interact with the hover.

Remove the mailKey column from PhabricatorPaste

In D25900#24197, @valerio.bozzolan wrote:

You know better than me the context. Is there a possibility that an installation is already without that column?

You know better than me the context. Is there a possibility that an installation is already without that column?

Created D25900 for the deleted paste mailkey migration.

Ran ./bin/storage upgrade --namespace phabricator_quickstart_test2, and that didn't fail.

OK lol no, this has nothing to do with D25898. Need another task for the "Missing input descriptions"

In D25898#24165, @avivey wrote:

• There's already a "description" field on these, which I think was supposed to be displayed; We might have lost it at some point, which is concerning.

• There's already a "description" field on these, which I think was supposed to be displayed; We might have lost it at some point, which is concerning.
• Use pht() for all human-visible text, to allow translation.

Need to go over the relevant code for deciding if a commit has an approved Revision - I think it looks mostly on the commit message.

I updated this revision so the title will only show the monogram if the user is logged in

• The title will only show the monogram when the post is public

have an approved Revision", but this too can be tricked by a sufficiently competent user

Hackerman meme

Don't find a problem with this at all.