ExtensionPage: rename "Uninstall" to "Purge" #3123
Conversation
askvortsov1
requested review from
tankerkiller125,
dsevillamartin and
davwheat
| 'uninstall', | ||
| <Button icon="fas fa-trash-alt" className="Button Button--primary" onclick={uninstall.bind(this)}> | ||
| {app.translator.trans('core.admin.extension.uninstall_button')} | ||
| 'purge', | ||
| <Button icon="fas fa-trash-alt" className="Button Button--primary" onclick={purge.bind(this)}> | ||
| {app.translator.trans('core.admin.extension.purge_button')} |
There was a problem hiding this comment.
I don't know of any extension that might be (if any) hooking into this, but we should probably avoid changing item list keys, as that'd be an easy breaking change.
There was a problem hiding this comment.
Any objections to keeping the old names now, and changing them with 2.0? Alternatively, we could add a deprecated dummy entry.
There was a problem hiding this comment.
Any objections to keeping the old names now, and changing them with 2.0?
Sounds good to me
Alternatively, we could add a deprecated dummy entry.
That could work too, then we'd remove it in 2.0, I'm fine with either solutions
There was a problem hiding this comment.
A dummy entry wouldn't make any difference, I don't believe. All our ItemList methods fail silently.
I think the bigger issue was that if someone was hiding this option for some reason, changing the key would make it re-appear. A dummy entry wouldn't change this behaviour.
There was a problem hiding this comment.
A dummy entry wouldn't make any difference, I don't believe. All our ItemList methods fail silently.
I think the bigger issue was that if someone was hiding this option for some reason, changing the key would make it re-appear. A dummy entry wouldn't change this behaviour.
Fair enough
Fixes #2965
Changes proposed in this pull request:
Title says it all.
Reviewers should focus on:
Should we retain the translation key names? It's not completely breaking (things will still work), but lang packs will need to update their translations.
Screenshot
Necessity
Confirmed
composer test).