829933 - http://people.mozilla.org/~catlee/highscores/highscores.html should state the trychooser syntax used

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P4)

Description

[Ed Morley [:emorley]]

In order to help determine TryServer abusers, it would be helpful if http://people.mozilla.org/~catlee/highscores/highscores.html stated the trychooser syntax used for each push shown when you expand the row for a user.

Comment 1

[Ed Morley [:emorley]]

If we could also include a more obvious indication that the per user rows are expandable - that would be great :-)

Comment 2

[Chris AtLee [:catlee]]

I'll take this opportunity to turn this into a really supported dashboard then. Right now it's being updated off of my laptop :)

Comment 3

[Ed Morley [:emorley]]

:-D

Comment 4

[Anhad Jai Singh (:ffledgling)]

Attachment: 0001-Bug-829933-reports-reportor-daily-highscores-highsco.patch

Patch adds try syntax with individual pushes.

I've manged to get the try syntax to display next to the revision, but I don't think I have the HTML/CSS for it completely down yet.
It looks particularly bad in cases where there's a long try syntax.

Chris, any feedback on how to improve this would be much appreciated. Thank you!

Running version here: http://web.iiit.ac.in/~anhadjai.singh/mozilla/highscores.html
(Runing off of a cached highscores.json at the moment)

Comment 5

[Chris AtLee [:catlee]]

Comment on attachment 819402 [details] [diff] [review]
0001-Bug-829933-reports-reportor-daily-highscores-highsco.patch

Review of attachment 819402 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks, this is a great start!

How does it look if you put the try syntax for each commit in the same <td> cell as the revision, but a line down? e.g.

[e-mail].....................	        2,063	5,022
                                 eeb9a6e52988	100	394
  try: -b o -p android -u mochitest-7 -t none

Ed, did you have any thoughts on how it should be presented?

::: reports/highscores/highscores.css
@@ +52,5 @@
> +    color: yellow;
> +    text-align: left;
> +    width: 70%;
> +    float:left;
> +}

small nit - I'd name this something like "try_syntax" or something similar.

::: reports/highscores/highscores.html
@@ +4,4 @@
>  <meta charset="utf-8">
>  <link rel="stylesheet" href="highscores.css">
>  <link href='https://fonts.googleapis.com/css?family=Michroma|Press+Start+2P' rel='stylesheet' type='text/css'>
> +<link href='http://fonts.googleapis.com/css?family=Roboto+Condensed:300' rel='stylesheet' type='text/css'>

this needs to be https:// to stop from getting mixed content warnings. is it being used anywhere?

::: reports/highscores/highscores.py
@@ +80,4 @@
>              # Figure out how much time we spent on this push
>              try:
> +                try_time, num_jobs = get_job_info("try", p['changesets'][-1]['node'][:12])
> +                try_syntax = re.search("try:.*$", p['changesets'][-1]['desc']).group(0)

this will raise an exception if the try syntax isn't found. can you make it behave gracefully in this case?

@@ +80,5 @@
>              # Figure out how much time we spent on this push
>              try:
> +                try_time, num_jobs = get_job_info("try", p['changesets'][-1]['node'][:12])
> +                try_syntax = re.search("try:.*$", p['changesets'][-1]['desc']).group(0)
> +                revisions.append( {"hours": try_time/3600.0, "revision": p['changesets'][-1]['node'][:12], "jobs": num_jobs, "try": try_syntax} )

shouldn't this be "try_syntax": try_syntax?

Comment 6

[Anhad Jai Singh (:ffledgling)]

Attachment: 0002-Bug-829933-reports-reportor-daily-highscores-highsco.patch

Fixed nits.
Changed the appearance, looks a bit better now.

Chris, Ed, please take a look and tell me if any other improvements are required. Thanks!

Newer running version uploaded to http://web.iiit.ac.in/~anhadjai.singh/mozilla/highscores.html

Comment 7

[Chris AtLee [:catlee]]

Comment on attachment 820033 [details] [diff] [review]
0002-Bug-829933-reports-reportor-daily-highscores-highsco.patch

Review of attachment 820033 [details] [diff] [review]:
-----------------------------------------------------------------

::: reports/highscores/highscores.py
@@ +81,5 @@
>              # Figure out how much time we spent on this push
>              try:
> +                try_time, num_jobs = get_job_info("try", p['changesets'][-1]['node'][:12])
> +                try_match = re.search("try:.*$", p['changesets'][-1]['desc'])
> +                if try_regex:

itym 'if try_match'? try_regex isn't defined here.

Comment 8

[Anhad Jai Singh (:ffledgling)]

Attachment: 0003-Bug-829933-reports-reportor-daily-highscores-highsco.patch

Sorry for the carelessness.
Patch with nit fixed.

Comment 9

[Chris AtLee [:catlee]]

Comment on attachment 820452 [details] [diff] [review]
0003-Bug-829933-reports-reportor-daily-highscores-highsco.patch

https://github.com/catlee/reportor/commit/02c6d5196104283e66ed980e2eb6517562264b6a

Comment 10

[Chris AtLee [:catlee]]

Thanks! This is live here: https://secure.pub.build.mozilla.org/builddata/reports/reportor/daily/highscores/highscores.html

Comment 11

[Ed Morley [:emorley]]

Looks good, thank you :-)