Skip to content

Bug 1465588 - Remove dead perf dashboard code part2#4222

Merged
sarah-clements merged 3 commits intomozilla:masterfrom
sarah-clements:remove-perf-code-part2
Nov 5, 2018
Merged

Bug 1465588 - Remove dead perf dashboard code part2#4222
sarah-clements merged 3 commits intomozilla:masterfrom
sarah-clements:remove-perf-code-part2

Conversation

@sarah-clements
Copy link
Copy Markdown
Contributor

I missed removing a few things (baseTitle, newTitle, phReleaseBlocker, etc), as mentioned in this pr and detailed in this commit message. Also, I dug deeper into the code and realized that trendtable.html is not being used because the remaining partials that use the ph-component-table component don't have a type attribute of 'trend'.

Tested the compare view locally, including the different filters and it works as expected.

remove baseTitle and newTitle from compare.js and into comparetable.html partial
remove releaseBlockerCriteria from compare.js and comparetable partial
remove other 'blocker' references from compare.js
@sarah-clements sarah-clements requested review from edmorley, jmaher and wlach and removed request for jmaher November 2, 2018 21:46
@jmaher
Copy link
Copy Markdown
Collaborator

jmaher commented Nov 2, 2018

things look good from my point of view

@sarah-clements sarah-clements removed the request for review from wlach November 2, 2018 23:30
Copy link
Copy Markdown
Contributor

@edmorley edmorley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome - even less to convert to React! :-)

I spotted one other remaining piece - after the removal here:
https://github.com/mozilla/treeherder/pull/4211/files#diff-5106b6d3e8e25463e577b92199db8864L138

getCounterMap() is never passed the forth argument (blockerCriteria), so that argument and its usage inside the function can be removed too (the reference to isBlocker in that function's comment should also be removed):

getCounterMap: function getDisplayLineData(testName, originalData, newData, blockerCriteria) {

if (blockerCriteria !== undefined &&
blockerCriteria[testName] !== undefined &&
cmap.isRegression &&
cmap.deltaPercentage > blockerCriteria[testName]) {
cmap.isBlocker = true;
} else {
cmap.isBlocker = false;
}

Comment thread ui/js/components/perf/compare.js
@edmorley
Copy link
Copy Markdown
Contributor

edmorley commented Nov 5, 2018

Looks great! Between this and #4211 there were 740 lines removed 😃

@sarah-clements sarah-clements merged commit e590908 into mozilla:master Nov 5, 2018
@sarah-clements sarah-clements deleted the remove-perf-code-part2 branch November 5, 2018 19:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants