Remove unused code in ManiphestReportController::renderBurn()
Needs ReviewPublic
Actions

Authored by aklapper on Tue, Mar 4, 11:03.

Details

Reviewers

None

Group Reviewers

O1: Blessed Committers

Maniphest Tasks

T16005: Remove unused table code in ManiphestReportController::renderBurn()

Summary

ManiphestReportController (which renders /maniphest/report/burn/) wastes CPU cycles. Thus remove useless code.

It tries to construct table rows never to be shown: The code calculates rows and adds them via $table = new AphrontTableView($rows), then defines $panel = new PHUIObjectBoxView() and does $panel->setTable($table) but a bit later $panel = id(new PhabricatorProjectBurndownChartEngine()) overwrites that table and all its data.
This re-definition of $panel was introduced in rP5c1b91ab457db9f3db10d8cc5e07831512645ebb.

The code creates a PhabricatorProjectBurndownChartEngine anyway, similar to PhabricatorProjectReportsController. So none of those expensive database queries are needed at all. Variables like list($burn_x, $burn_y) = $this->buildSeries($data); are never read since rPf8ebc71b8f217ed156f416ddb4cd028dcaa28174.

Bug: T16005

Test Plan

Make sure /applications/view/PhabricatorFactApplication/ is installed
Go to /maniphest/report/burn/ for global view, go to some URL like /maniphest/report/burn/?project=PHID-PROJ-1234567890abcdef for per-projcet view
See that the "Burnup Rate" chart is rendered in the same way before and after applying this patch
Optionally, run ./bin/cache purge --all (but looks like no charts are cached?) and reload
Optionally, go to /project/reports/1/ and as expected the burndown chart is the same as in step 2

Diff Detail

Repository

rP Phorge

Branch

T16005ChartPerform (branched from master)

Lint

Lint Passed

Unit

Tests Passed

Build Status

Buildable 1766
Build 1766: arc lint + arc unit

Event Timeline

aklapper created this revision.Tue, Mar 4, 11:03

Owners added a reviewer: O1: Blessed Committers.Tue, Mar 4, 11:03

Herald added subscribers: Cigaryno, Matthew, valerio.bozzolan, tobiaswiese. · View Herald TranscriptTue, Mar 4, 11:03

aklapper requested review of this revision.Tue, Mar 4, 11:03

Harbormaster completed remote builds in B1762: Diff 2720.Tue, Mar 4, 11:03

aklapper retitled this revision from Remove unused code in ManiphestReportController:renderBurn() to Remove unused code in ManiphestReportController::renderBurn().Tue, Mar 4, 11:06

It seems that there is even more code to rip out here

Rip out more unused ancient code like unneeded expensive database queries

Harbormaster completed remote builds in B1766: Diff 2725.Thu, Mar 6, 09:56

aklapper edited the summary of this revision. (Show Details)Thu, Mar 6, 10:01

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