Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2255293003:
[Findit] On hover over, shows pass rate for a swarming rerun (Closed)
DescriptionOn hover over, shows pass rate for a swarming rerun.
Guide on tooltip hovering found: http://www.flotcharts.org/flot/examples/interacting/
BUG=617808
Committed: https://chromium.googlesource.com/infra/infra/+/66438fbc9e2e56956125638f02837a92c5322812
Patch Set 1 #
Total comments: 12
Patch Set 2 : addressed comments #
Total comments: 4
Patch Set 3 : addressed comments #Patch Set 4 : git pull #Patch Set 5 : fixed test #
Messages
Total messages: 24 (11 generated)
caiw@google.com changed reviewers: + chanli@chromium.org, lijeffrey@chromium.org, stgao@chromium.org
Description was changed from ========== On hover over, shows pass rate for a swarming rerun BUG=617808 ========== to ========== On hover over, shows pass rate for a swarming rerun. Guide on tooltip hovering found: http://www.flotcharts.org/flot/examples/interacting/ BUG=617808 ==========
Great, this CL will make the trend diagram more useful. https://codereview.chromium.org/2255293003/diff/1/appengine/findit/templates/... File appengine/findit/templates/flake/result.html (right): https://codereview.chromium.org/2255293003/diff/1/appengine/findit/templates/... appengine/findit/templates/flake/result.html:32: }) nit: end with ";"? https://codereview.chromium.org/2255293003/diff/1/appengine/findit/templates/... appengine/findit/templates/flake/result.html:46: y = item.datapoint[1].toFixed(2); nit: one statement in one line. https://codereview.chromium.org/2255293003/diff/1/appengine/findit/templates/... appengine/findit/templates/flake/result.html:48: $("#tooltip").html("Build Number " + x + " has a pass rate of " + y) How about this format instead? Build Number: XXXX Pass rate: YYYY https://codereview.chromium.org/2255293003/diff/1/appengine/findit/templates/... appengine/findit/templates/flake/result.html:59: <body> It seems a good idea to show the master name/ builder name/ build number/step name/test name on the page too. https://codereview.chromium.org/2255293003/diff/1/appengine/findit/templates/... appengine/findit/templates/flake/result.html:68: Suspected Build Number of Culprit: {{suspected_flake_build_number}} Wording nit: "Suspected Build Number of Culprit:" --> "Regressed in Build:"?
https://codereview.chromium.org/2255293003/diff/1/appengine/findit/templates/... File appengine/findit/templates/flake/result.html (right): https://codereview.chromium.org/2255293003/diff/1/appengine/findit/templates/... appengine/findit/templates/flake/result.html:43: $("#placeholder").bind("plothover", function (event, pos, item) { Currently, it is hard to tell where data points are. Is it possible to enlarge them just like those in the sample http://www.flotcharts.org/flot/examples/interacting/?
One suggestion: since we don't have result for each an every build, maybe we should display the dots where results exist. Otherwise users may wonder why can't they check result for certain build.
https://codereview.chromium.org/2255293003/diff/1/appengine/findit/templates/... File appengine/findit/templates/flake/result.html (right): https://codereview.chromium.org/2255293003/diff/1/appengine/findit/templates/... appengine/findit/templates/flake/result.html:32: }) On 2016/08/19 04:14:04, stgao wrote: > nit: end with ";"? Done. https://codereview.chromium.org/2255293003/diff/1/appengine/findit/templates/... appengine/findit/templates/flake/result.html:43: $("#placeholder").bind("plothover", function (event, pos, item) { On 2016/08/19 04:22:13, stgao wrote: > Currently, it is hard to tell where data points are. Is it possible to enlarge > them just like those in the sample > http://www.flotcharts.org/flot/examples/interacting/ Done. https://codereview.chromium.org/2255293003/diff/1/appengine/findit/templates/... appengine/findit/templates/flake/result.html:46: y = item.datapoint[1].toFixed(2); On 2016/08/19 04:14:04, stgao wrote: > nit: one statement in one line. Done. https://codereview.chromium.org/2255293003/diff/1/appengine/findit/templates/... appengine/findit/templates/flake/result.html:48: $("#tooltip").html("Build Number " + x + " has a pass rate of " + y) On 2016/08/19 04:14:04, stgao wrote: > How about this format instead? > > Build Number: XXXX > Pass rate: YYYY Done. https://codereview.chromium.org/2255293003/diff/1/appengine/findit/templates/... appengine/findit/templates/flake/result.html:59: <body> On 2016/08/19 04:14:04, stgao wrote: > It seems a good idea to show the master name/ builder name/ build number/step > name/test name on the page too. Done. https://codereview.chromium.org/2255293003/diff/1/appengine/findit/templates/... appengine/findit/templates/flake/result.html:68: Suspected Build Number of Culprit: {{suspected_flake_build_number}} On 2016/08/19 04:14:04, stgao wrote: > Wording nit: "Suspected Build Number of Culprit:" --> "Regressed in Build:"? Done.
lgtm with nits. https://codereview.chromium.org/2255293003/diff/20001/appengine/findit/templa... File appengine/findit/templates/flake/result.html (right): https://codereview.chromium.org/2255293003/diff/20001/appengine/findit/templa... appengine/findit/templates/flake/result.html:29: show: true nit: indent https://codereview.chromium.org/2255293003/diff/20001/appengine/findit/templa... appengine/findit/templates/flake/result.html:52: var x = Math.round(item.datapoint[0].toFixed(2)) nit: end with ";".
https://codereview.chromium.org/2255293003/diff/20001/appengine/findit/templa... File appengine/findit/templates/flake/result.html (right): https://codereview.chromium.org/2255293003/diff/20001/appengine/findit/templa... appengine/findit/templates/flake/result.html:29: show: true On 2016/08/19 20:30:34, stgao wrote: > nit: indent Done. https://codereview.chromium.org/2255293003/diff/20001/appengine/findit/templa... appengine/findit/templates/flake/result.html:52: var x = Math.round(item.datapoint[0].toFixed(2)) On 2016/08/19 20:30:34, stgao wrote: > nit: end with ";". Done.
The CQ bit was checked by caiw@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from stgao@chromium.org Link to the patchset: https://codereview.chromium.org/2255293003/#ps40001 (title: "addressed comments")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: Infra Linux Precise 32 Tester on luci.infra.try (JOB_FAILED, https://luci-milo.appspot.com/swarming/task/30bdde0618ef5b10) Infra Presubmit on luci.infra.try (JOB_FAILED, https://luci-milo.appspot.com/swarming/task/30bdde06a94c3310)
The CQ bit was checked by caiw@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from stgao@chromium.org Link to the patchset: https://codereview.chromium.org/2255293003/#ps60001 (title: "git pull")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: Infra Linux Precise 32 Tester on luci.infra.try (JOB_FAILED, https://luci-milo.appspot.com/swarming/task/30bdfd578b918c10)
The CQ bit was checked by caiw@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from stgao@chromium.org Link to the patchset: https://codereview.chromium.org/2255293003/#ps80001 (title: "fixed test")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== On hover over, shows pass rate for a swarming rerun. Guide on tooltip hovering found: http://www.flotcharts.org/flot/examples/interacting/ BUG=617808 ========== to ========== On hover over, shows pass rate for a swarming rerun. Guide on tooltip hovering found: http://www.flotcharts.org/flot/examples/interacting/ BUG=617808 Committed: https://chromium.googlesource.com/infra/infra/+/66438fbc9e2e56956125638f02837... ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001) as https://chromium.googlesource.com/infra/infra/+/66438fbc9e2e56956125638f02837... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
