Remove unused methods in PhabricatorEditorExtension
ClosedPublic
Actions

Authored by aklapper on May 16 2024, 10:07.

Details

Reviewers

valerio.bozzolan

Group Reviewers

O1: Blessed Committers

Commits

rPb0bba4d14235: Remove unused methods in PhabricatorEditorExtension

Summary

The methods newRequiredTransasctionError() [sic!] and newInvalidTransactionError() are not called anywhere in the codebase. Both are final protected, thus remove them.

Incidentally, speedup Diviner generation by at least 0.000000000000000000000001 milliseconds.

Test Plan

Grep the code.

Diff Detail

Repository

rP Phorge

Branch

NEeditorExtensionNewError (branched from master)

Lint

Lint Passed

Unit

Tests Passed

Build Status

Buildable 1410
Build 1410: arc lint + arc unit

Event Timeline

aklapper created this revision.May 16 2024, 10:07

Owners added a reviewer: O1: Blessed Committers.May 16 2024, 10:07

Herald added subscribers: Cigaryno, Matthew, valerio.bozzolan, tobiaswiese. · View Herald TranscriptMay 16 2024, 10:07

aklapper requested review of this revision.May 16 2024, 10:07

Harbormaster completed remote builds in B1264: Diff 1994.May 16 2024, 10:07

Also fix the typo in the function name

this function does not seem to get called anywhere anyway

Since 2019, this method is whispering "master... please... kill me..."

So, please respect their wish, and kill it with maximum violence.

valerio.bozzolan awarded a token.Jun 5 2024, 15:00

kill it

This stuff was introduced in rPf7e8fa0764fe7bd2546442cccbb6ec447b0ea8e2 so I initially thought the consumers were meant to be potential external editor code but I might be wrong because they are final protected. Indeed newRequiredTransactionError and newInvalidTransactionError are not called anywhere within Phorge itself, so if you confirm I am fine with removing both.

@valerio.bozzolan Any thoughts/opinion regarding my last comment? :) TIA

Feel free to kill this method violently 🗡 since as you said this is protected (no external usages) + final (no overrides) and visibly broken since 5 years

(Fun fact, to type the dagger emojii I tiped :D since I always type :D obtaining a dagger by mistake. Now it's a feature. asd - T15285)

Remove unused final protected methods in PhabricatorEditorExtension

Harbormaster completed remote builds in B1410: Diff 2201.Jul 2 2024, 12:39

aklapper retitled this revision from Fix call to non-existing newError() in PhabricatorEditorExtension to Remove unused final protected methods in PhabricatorEditorExtension.Jul 2 2024, 12:42

aklapper edited the summary of this revision. (Show Details)

aklapper edited the test plan for this revision. (Show Details)

lgtm

This revision is now accepted and ready to land.Jul 2 2024, 14:07

valerio.bozzolan edited the summary of this revision. (Show Details)Jul 2 2024, 14:07

aklapper retitled this revision from Remove unused final protected methods in PhabricatorEditorExtension to Remove unused methods in PhabricatorEditorExtension.Jul 2 2024, 15:54

Closed by commit rPb0bba4d14235: Remove unused methods in PhabricatorEditorExtension. · Explain WhyJul 2 2024, 15:55

This revision was automatically updated to reflect the committed changes.

aklapper added a commit: rPb0bba4d14235: Remove unused methods in PhabricatorEditorExtension.