Closed Bug 1134780 Opened 9 years ago Closed 9 years ago

when adding dataseries to perfherder, I would like to take a data point and add other platforms easily

Categories

(Tree Management :: Perfherder, defect)

x86_64
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jmaher, Assigned: sabergeass)

Details

Attachments

(2 files)

One common feature in investigating regressions is to see what the effect of a given change has.  When we get an alert or see an issue on a graph, it is common to look on all related platforms to see if this is a problem there as well.

The current add data series UI is not designed for this, but maybe we could have a button/icon in the dataseries on the left "add dataseries for all platforms".  This would mean that if we did:
mozilla-inbound
windows 7
ts paint

We would have an easy way (I am suggesting a link/button in the data series description, but that is all I can think of right now) to add all ts paint data series on mozilla-inbound for the platforms we run (winxp, win7, win8, 10.6, 10.8, linux32, linux64, android 4)

This would result in a list of dataseries which I would then turn on/off to examine the impact and keep a list of the ones which are affected to share with others.
(In reply to Joel Maher (:jmaher) from comment #0)
> We would have an easy way (I am suggesting a link/button in the data series
> description, but that is all I can think of right now) to add all ts paint
> data series on mozilla-inbound for the platforms we run (winxp, win7, win8,
> 10.6, 10.8, linux32, linux64, android 4)

Hi Joel, maybe we could add a link/button(just like you said) under the dataseries. After people click it, a modal(http://angular-ui.github.io/bootstrap/) will show itself and contain  all platforms data we want. Maybe more like a view combine test chooser(the modal part) and graph view(the data part).
Attachment #8637712 - Flags: feedback?(jmaher)
I like it.

Putting more thought into this if we make this more than a single click, we could do:
"view similar data"

which would then give you a list of:
platforms - and select what you want
branches - select what you want

I don't think we should allow all platforms/branches together- it would be too cluttered, but adding 3 branches easily would be nice, or adding 4 or 5 branches would be nice.

I guess seeing how the modal dialog works would help determine which route to go.
(In reply to Joel Maher (:jmaher) from comment #2)
> I like it.
> 
> Putting more thought into this if we make this more than a single click, we
> could do:
> "view similar data"
> 
> which would then give you a list of:
> platforms - and select what you want
> branches - select what you want
> 
> I don't think we should allow all platforms/branches together- it would be
> too cluttered, but adding 3 branches easily would be nice, or adding 4 or 5
> branches would be nice.
> 
> I guess seeing how the modal dialog works would help determine which route
> to go.

I would probably implement this by popping up the existing test chooser pre-filled with a set of data. That way you wouldn't have to add/debug a whole bunch of new UI.
Attachment #8637712 - Flags: feedback?(jmaher)
if we were to fill the existing test chooser, then I would like to ensure we have two easy options:
* "add all related platforms"
* "add all related branches"

those are two different use cases, but would be solved in almost the same way.
(In reply to Joel Maher (:jmaher) from comment #4)
> if we were to fill the existing test chooser, then I would like to ensure we
> have two easy options:
> * "add all related platforms"
> * "add all related branches"
> 
> those are two different use cases, but would be solved in almost the same
> way.

Cool, I will try to work on it.
Assignee: nobody → sabergeass
Attached file PR for bug 1134780
I meet some problem when I want to add all related branches. So, I made some comments on github and ask for help. :Will Could you tell me how to deal with it? And please tell me whether those code I had wrote fit your mind. Thank you!
Attachment #8639293 - Flags: feedback?(wlachance)
Comment on attachment 8639293 [details] [review]
PR for bug 1134780

I like where this is going! See PR for some suggestions.
Attachment #8639293 - Flags: feedback?(wlachance) → feedback+
Comment on attachment 8639293 [details] [review]
PR for bug 1134780

Hm, I know it's not good enough to review or ask test for it, but I need some suggest and directions to tell me how to improve this PR until it's good to go. So, I decide to ask feedback once again. :wlach Please tell me what's you think about it. Thank you very much! :)
Attachment #8639293 - Flags: feedback+ → feedback?(wlachance)
Comment on attachment 8639293 [details] [review]
PR for bug 1134780

Gave a bit of advice on how to move ahead in the PR, hopefully it makes sense!
Attachment #8639293 - Flags: feedback?(wlachance)
Attachment #8639293 - Flags: review?(wlachance)
Comment on attachment 8639293 [details] [review]
PR for bug 1134780

This is on its way but needs a bit of work. I gave some preliminary comments and a large suggestion for improvement (use promises) in the bug. Looking forward to the next round of this patch. Thanks for your patience, as always!
Attachment #8639293 - Flags: review?(wlachance) → review-
Attachment #8639293 - Flags: review- → review?(wlachance)
Hi Will,

I just ask review for let you know I have address my PR and would like to move on to the next bug :) So please be free *not* to review and comment on it until you finish those work on your hand(bug 1192976 or other related bug). However, some places I haven't change as your told me on github comment because I haven't figure how to apply them or the way I use can achieved the same effect as well.

Thank you very much, thank you for your patience and selfless help. Please tell me if you think there are anything could be improved. :)
Comment on attachment 8639293 [details] [review]
PR for bug 1134780

Sorry for the wait on this one. In general, I like how this works and it seems to work well in my testing. There are a few small changes I'd like to see, after that I'd be happy to land the patch.
Attachment #8639293 - Flags: review?(wlachance) → review-
Attachment #8639293 - Flags: review- → review?(wlachance)
Comment on attachment 8639293 [details] [review]
PR for bug 1134780

One last change required before we can land.
Attachment #8639293 - Flags: review?(wlachance) → review-
Attachment #8639293 - Flags: review- → review?(wlachance)
Comment on attachment 8639293 [details] [review]
PR for bug 1134780

There was one last small problem (add related branches vs. add related platforms were swapped) which I fixed before pushing. Thanks! This should be a very useful feature.
Attachment #8639293 - Flags: review?(wlachance) → review+
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: