Skip to content
This repository has been archived by the owner on Nov 10, 2017. It is now read-only.

Bug 1142680 - perfherder compare subtest view #485

Closed
wants to merge 3 commits into from
Closed

Bug 1142680 - perfherder compare subtest view #485

wants to merge 3 commits into from

Conversation

jmaher
Copy link

@jmaher jmaher commented Apr 23, 2015

initial work with some refactoring to get subtests viewed from the compare view on perfherder.

Review on Reviewable

@@ -6,6 +6,19 @@

var perf = angular.module("perf", ['ui.router', 'ui.bootstrap', 'treeherder']);

perf.factory('isReverseTest', [ function() {
Copy link

Choose a reason for hiding this comment

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

We should really be picking this up in the signature. Filed https://bugzilla.mozilla.org/show_bug.cgi?id=1158233 for this

var options = [ optionCollectionMap[signatureProps.option_collection_hash] ];
if (e10s) {
options.push("e10s");
perf.factory('phSeries', ['$http', 'thServiceDomain', function($http, thServiceDomain) {
Copy link

Choose a reason for hiding this comment

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

For whatever reason, the convention seems to be to capitalize factories. I.e. phSeries should be PhSeries.

@jmaher
Copy link
Author

jmaher commented Apr 24, 2015

thanks for the awesome feedback, I have updated the commit with --amend and all items should be addressed. This feels a lot cleaner now- not perfect, but many workable!

}]);


perf.factory('getResultsMap', [ '$q', '$http', 'thServiceDomain', 'phSeries', 'math',
Copy link

Choose a reason for hiding this comment

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

Let's fold getCounterMap, getResultsMap into a factory called PhCompare -- they're only really useful in that context.

@jmaher
Copy link
Author

jmaher commented Apr 24, 2015

updated PR with --amend, this should be rad!

@wlach
Copy link

wlach commented Apr 24, 2015

Merged manually.

@wlach wlach closed this Apr 24, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants