Bug 1465588 - Remove unused Perfherder dashboard code#4211
Bug 1465588 - Remove unused Perfherder dashboard code#4211sarah-clements merged 2 commits intomozilla:masterfrom
Conversation
edmorley
left a comment
edmorley
left a comment
There was a problem hiding this comment.
Yey for deleting dead code! :-D
| import chunk from 'lodash/chunk'; | ||
|
|
||
| import perf from '../../perf'; | ||
| import { thDefaultRepo, phBlockers, phTimeRanges } from '../../../helpers/constants'; |
There was a problem hiding this comment.
The phBlockers constant is now unused so can be removed too
| test-list="testList" | ||
| compare-results="compareResults" | ||
| filter-options="filterOptions" | ||
| release-blocker-criteria="1"> |
There was a problem hiding this comment.
I believe release-blocker-criteria corresponds to the releaseBlockerCriteria references in some of the remaining files (I hate Angular's automatic naming conversion) - which means they are now unused after this template is deleted. As such I think anything to do with "blockers" (eg releaseBlockerCriteria, showOnlyBlockers, isBlocker) can now be deleted from compare.js, the compare templates and a few other places.
There was a problem hiding this comment.
This review comment was not addressed?
There was a problem hiding this comment.
Addressed below in the comment about newTitle and baseTitle :)
Edit: sorry, misunderstood your comment. Will look into it again and add this to a follow up pr.
| <div ng-if="!dataLoading && !revisionNotFound"> | ||
| <ph-compare-table | ||
| base-title={{baseTitle}} | ||
| new-title={{variantTitle}} |
There was a problem hiding this comment.
Similarly, I believe all newTitle and baseTitle references can now also be removed, since the standard compare views don't use them.
There was a problem hiding this comment.
It seems they're used in the trendtable.html and comparetable.html partials - either of which the phCompareTable component uses so leaving in for now (same applies to the blockers).
There was a problem hiding this comment.
The baseTitle and newTitle variables allowed for overriding of the headings on those pages, however nothing overrides them any more, so they are always at default values. As such the default values can be moved from here:
treeherder/ui/js/components/perf/compare.js
Lines 30 to 35 in debac0c
...to the templates, and then baseTitle and newTitle removed.
There was a problem hiding this comment.
I see - thanks for clarifying.
wlach
left a comment
wlach
left a comment
There was a problem hiding this comment.
lgtm assuming everything still works! Ed's suggestions look right to me.
4 participants
No description provided.