Page MenuHomePhabricator

Remove "metamta.*.subject-prefix" options
ClosedPublic

Authored by epriestley on Jan 17 2019, 8:12 PM.

Details

Summary

In ~2012, the first of these options was added because someone who hates dogs and works at Asana also hated [Differential] in the subject line. The use case there was actually removing the text, not changing it, but I made the prefix editable since it seemed like slightly less of a one-off.

These options are among the dumbest and most useless config options we have and very rarely used, see T11760. A very small number of instances have configured one of these options.

Newer applications stopped providing these options and no one has complained.

You can get the same effect with translation.override. Although I'm not sure we'll keep that around forever, it's a reasonable replacement today. I'll call out an example in the changelog to help installs that want to preserve this option.

If we did want to provide this, it should just be in ApplicationsSettings for each application, but I think it's wildly-low-value and "hack via translations" or "local patch" are entirely reasonable if you really want to change these strings.

Test Plan

Grepped for subject-prefix.

Diff Detail

Repository
rP Phabricator
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

epriestley created this revision.Jan 17 2019, 8:12 PM
epriestley edited the summary of this revision. (Show Details)
epriestley requested review of this revision.Jan 17 2019, 8:14 PM

I'm really going to miss this code:

src/applications/conpherence/config/ConpherenceConfigOptions.php
3

Wow.

src/applications/legalpad/config/PhabricatorLegalpadConfigOptions.php
3

Wow.

src/applications/macro/config/PhabricatorMacroConfigOptions.php
3

Wow.

src/applications/paste/config/PhabricatorPasteConfigOptions.php
3

Wow.

src/applications/pholio/config/PhabricatorPholioConfigOptions.php
3

Wow.

someone who hates dogs

Actually, I'm completely wrong. The villain responsible for this is: @epriestley, in D291. I am to blame.

I'm pretty sure there was some other change around "setting it to nothing should remove it entirely" but I couldn't dig that up immediately.

amckinley accepted this revision.Jan 17 2019, 9:53 PM
amckinley added inline comments.
src/applications/conpherence/editor/ConpherenceEditor.php
241–243

Just for giggles, why don't we implement getMailSubjectPrefix in PhabricatorApplicationTransactionEditor and just have it do something like this:

return '[' . $this->getApplication()->getApplicationName() . ']'
This revision is now accepted and ready to land.Jan 17 2019, 9:53 PM
epriestley added inline comments.Jan 17 2019, 9:58 PM
src/applications/conpherence/editor/ConpherenceEditor.php
241–243

I'd probably like to get there eventually. Right now, it would mean:

  • You can't translation.override it, since it's no longer a distinct string.
  • Files would change from [File] to [Files].

I don't think either one is a big deal, but this moves us toward that without actually breaking anything.

(Some applications also have more than one object, e.g. SSHKeys send [SSH Key] rather than [Auth].)

The whole feature is also semi-mooted by the application(...) mail stamp.

This revision was automatically updated to reflect the committed changes.