id, vs and /new/ in URL all affect content of the generated file.

In D25478#13957, @speck wrote:

What do you think about keeping the current naming scheme but whitelisting a handful of query params to use instead of using them all? The timestamp adds noise and it's probably nice to see the ID of the diff in the filename, e.g. D25478.id1541.diff? Maybe just allow the the id parameter and regex that it's value is just a string of numbers.

Should this be configurable and behind a feature flag, as some Phorge installations may not want this (plus may slow down performance a little bit)?

In D25467#13453, @valerio.bozzolan wrote:

Uh! Nice!

Interestingly this CSS change only has effects on Chromium 119 and it does not change anything in my Mozilla Firefox 118. Can you give a quick look?

This is related to https://secure.phabricator.com/T13546. "classic" Arcanist workflows were being upgraded to Toolsets, but the migration was not complete. See comments starting with TOOLSETS: , e.g. in ArcanistConfigurationSource.php.

Phrasing

In D25464#13372, @speck wrote:

This will require documentation of some sort, specifically for the upgrade notes to indicate that if someone relies on rendering PDFs currently then after upgrading they would need to update that configuration.

I think making a task to document the issue and linking in the release notes would be a good approach and what upstream would typically have done.

In D25464#13328, @speck wrote:

The Referenced Files section of this diff looks like someone is looking for a vulnerability. Any idea what’s happening here?

arc diff throws an error and arc land ignores it. Instead of getting it to work with Differential, I'm amending the commit manually with git commit --amend -s after arc land --hold.

Update commit message

Update config description

Mozilla's fixes were quick but not in good quality for direct submission to Phorge. I have adapted or rewritten two of the changes into D25462 and D25464.

Before landing this change, I would like to know the right way to credit both the original author and my modifications with Arcanist (Q85).

Reword

@valerio.bozzolan If you didn't get an answer, try asking in #conduit. I didn't realize that # needs to be quoted in Remarkup. 😢

@valerio.bozzolan Upstream removed code that enforces $always_visible when $file->getIsProfileImage() is true in PhabricatorFileQuery.php, but I'm afraid that this change may break other things, so I did not apply it here, but on the other hand kept the description of "Image will be Public".

Mention upstream commit

It would be great if Mozilla's team could join forces with Phorge. Would you (the core team) contact them in Conduit on chat.mozilla.org and mozilla.slack.com?

Fix lint warnings

I have reviewed it and made some comments. On a remotely related topic, TLS handshakes are expensive and persistent connections can reduce latency and server load by reusing TLS connections, so maybe we should make it configurable outside of cluster.databases as well.