Adding gantt chart to Build Status page on Buildbot Masters

Adding gantt chart to Build Status page on Buildbot Masters

Screenshots:
Initial page load - https://screenshot.googleplex.com/NJjHFTcXU9T.png
After button clicked - https://screenshot.googleplex.com/QHn0DLd0Jv9.png

BUG=663245
TEST=local
 - ran local buildbot, generated screenshots

Committed: https://chromium.googlesource.com/chromium/tools/build/+/f128e42562e0a67c111394c5f398ef9b1d93b6bd

[philwright, 2016-11-07 22:08:18]

Description was changed from

==========
Adding gantt chart to Build Status page for tryserver.chromium.linux

BUG=
==========

to

==========
Adding gantt chart to Build Status page for tryserver.chromium.linux

BUG=
==========

[philwright, 2016-11-07 22:08:18]

philwright@google.com changed reviewers:
+ tansell@chromium.org

[philwright, 2016-11-08 03:43:35]

Description was changed from

==========
Adding gantt chart to Build Status page for tryserver.chromium.linux

BUG=
==========

to

==========
Adding gantt chart to Build Status page for tryserver.chromium.linux

BUG=663245
==========

[philwright, 2016-11-08 03:43:35]

philwright@google.com changed reviewers:
+ dsansome@chromium.org

[philwright, 2016-11-08 03:52:32]

Description was changed from

==========
Adding gantt chart to Build Status page for tryserver.chromium.linux

BUG=663245
==========

to

==========
Adding gantt chart to Build Status page for tryserver.chromium.linux

Screenshots:
Initial page load - https://screenshot.googleplex.com/NJjHFTcXU9T.png
After button clicked - https://screenshot.googleplex.com/QHn0DLd0Jv9.png

BUG=663245
==========

[philwright, 2016-11-08 03:57:32]

Description was changed from

==========
Adding gantt chart to Build Status page for tryserver.chromium.linux

Screenshots:
Initial page load - https://screenshot.googleplex.com/NJjHFTcXU9T.png
After button clicked - https://screenshot.googleplex.com/QHn0DLd0Jv9.png

BUG=663245
==========

to

==========
Adding gantt chart to Build Status page on Buildbot Masters

Screenshots:
Initial page load - https://screenshot.googleplex.com/NJjHFTcXU9T.png
After button clicked - https://screenshot.googleplex.com/QHn0DLd0Jv9.png

BUG=663245
TEST=local
 - ran local buildbot, generated screenshots
==========

[philwright, 2016-11-08 03:57:32]

philwright@google.com changed reviewers:
+ estaab@chromium.org, iannucci@chromium.org, martiniss@chromium.org,
sergiyb@chromium.org

[dsansome, 2016-11-08 04:49:12]

lgtm

Nice!

I'd like you to reconcile the differences between the various build.html files,
and if possible move them all to one directory.

https://codereview.chromium.org/2484593002/diff/120001/masters/master.chromiu...
File masters/master.chromium.perf.fyi/templates/build.html (right):

https://codereview.chromium.org/2484593002/diff/120001/masters/master.chromiu...
masters/master.chromium.perf.fyi/templates/build.html:12: {'name': '{{
s.getName() }}',
Do any step names contain ' characters?
Use the escapejs filter:
https://docs.djangoproject.com/en/1.10/ref/templates/builtins/#escapejs

https://codereview.chromium.org/2484593002/diff/120001/masters/master.chromiu...
File masters/master.chromium/public_html/gantt_chart.js (right):

https://codereview.chromium.org/2484593002/diff/120001/masters/master.chromiu...
masters/master.chromium/public_html/gantt_chart.js:1:
google.charts.load('current', {'packages':['gantt']});
Add a copyright header:

// Copyright 2016 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

https://codereview.chromium.org/2484593002/diff/120001/masters/master.chromiu...
masters/master.chromium/public_html/gantt_chart.js:3: /*
Use JSDoc style comments:
https://engdoc.corp.google.com/eng/doc/devguide/js/styleguide.shtml?cl=head#j...

https://codereview.chromium.org/2484593002/diff/120001/masters/master.chromiu...
masters/master.chromium/public_html/gantt_chart.js:21: var reconciledSteps = [];
s/var/let/

https://codereview.chromium.org/2484593002/diff/120001/masters/master.chromiu...
masters/master.chromium/public_html/gantt_chart.js:24: for (i = 0; i <
steps.length; i++) {
Use for...of
(https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/...)

for (let s of steps) {

https://codereview.chromium.org/2484593002/diff/120001/masters/master.chromiu...
masters/master.chromium/public_html/gantt_chart.js:27: isTriggerStep =
/\[trigger\] (.*)/.exec(s.name);
Always scope local variables using "let".

https://codereview.chromium.org/2484593002/diff/120001/masters/master.chromiu...
masters/master.chromium/public_html/gantt_chart.js:52: for (i = 0; i <
steps.length; i++) {
for..of

https://codereview.chromium.org/2484593002/diff/120001/masters/master.chromiu...
masters/master.chromium/public_html/gantt_chart.js:63: labelMaxWidth: 500
Trailing commas are OK now:
https://engdoc.corp.google.com/eng/doc/devguide/js/styleguide.shtml?cl=head#f...

https://codereview.chromium.org/2484593002/diff/120001/masters/master.chromiu...
masters/master.chromium/public_html/gantt_chart.js:68:
document.getElementById('chart_div')
4-space indentation here

https://codereview.chromium.org/2484593002/diff/120001/masters/master.chromiu...
masters/master.chromium/public_html/gantt_chart.js:69: );
And put this on the line above

[philwright, 2016-11-08 23:17:47]

The CQ bit was checked by philwright@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-11-08 23:17:59]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-08 23:57:11]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-08 23:57:12]

Dry run: Try jobs failed on following builders:
  Build Presubmit on luci.infra.try (JOB_FAILED,
https://luci-milo.appspot.com/swarming/task/325f0d86b3a53810)

[philwright, 2016-11-09 04:58:39]

Comments addressed

https://codereview.chromium.org/2484593002/diff/120001/masters/master.chromiu...
File masters/master.chromium.perf.fyi/templates/build.html (right):

https://codereview.chromium.org/2484593002/diff/120001/masters/master.chromiu...
masters/master.chromium.perf.fyi/templates/build.html:12: {'name': '{{
s.getName() }}',
On 2016/11/08 04:49:11, dsansome wrote:
> Do any step names contain ' characters?
> Use the escapejs filter:
> https://docs.djangoproject.com/en/1.10/ref/templates/builtins/#escapejs

Done.

https://codereview.chromium.org/2484593002/diff/120001/masters/master.chromiu...
File masters/master.chromium/public_html/gantt_chart.js (right):

https://codereview.chromium.org/2484593002/diff/120001/masters/master.chromiu...
masters/master.chromium/public_html/gantt_chart.js:1:
google.charts.load('current', {'packages':['gantt']});
On 2016/11/08 04:49:11, dsansome wrote:
> Add a copyright header:
> 
> // Copyright 2016 The Chromium Authors. All rights reserved.
> // Use of this source code is governed by a BSD-style license that can be
> // found in the LICENSE file.

Done.

https://codereview.chromium.org/2484593002/diff/120001/masters/master.chromiu...
masters/master.chromium/public_html/gantt_chart.js:3: /*
On 2016/11/08 04:49:11, dsansome wrote:
> Use JSDoc style comments:
>
https://engdoc.corp.google.com/eng/doc/devguide/js/styleguide.shtml?cl=head#j...

Done.

https://codereview.chromium.org/2484593002/diff/120001/masters/master.chromiu...
masters/master.chromium/public_html/gantt_chart.js:21: var reconciledSteps = [];
On 2016/11/08 04:49:11, dsansome wrote:
> s/var/let/

Done.

https://codereview.chromium.org/2484593002/diff/120001/masters/master.chromiu...
masters/master.chromium/public_html/gantt_chart.js:24: for (i = 0; i <
steps.length; i++) {
On 2016/11/08 04:49:11, dsansome wrote:
> Use for...of
>
(https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/...)
> 
> for (let s of steps) {

Done.

https://codereview.chromium.org/2484593002/diff/120001/masters/master.chromiu...
masters/master.chromium/public_html/gantt_chart.js:27: isTriggerStep =
/\[trigger\] (.*)/.exec(s.name);
On 2016/11/08 04:49:11, dsansome wrote:
> Always scope local variables using "let".

Done.

https://codereview.chromium.org/2484593002/diff/120001/masters/master.chromiu...
masters/master.chromium/public_html/gantt_chart.js:52: for (i = 0; i <
steps.length; i++) {
On 2016/11/08 04:49:11, dsansome wrote:
> for..of

Done.

https://codereview.chromium.org/2484593002/diff/120001/masters/master.chromiu...
masters/master.chromium/public_html/gantt_chart.js:63: labelMaxWidth: 500
On 2016/11/08 04:49:11, dsansome wrote:
> Trailing commas are OK now:
>
https://engdoc.corp.google.com/eng/doc/devguide/js/styleguide.shtml?cl=head#f...

Done.

https://codereview.chromium.org/2484593002/diff/120001/masters/master.chromiu...
masters/master.chromium/public_html/gantt_chart.js:68:
document.getElementById('chart_div')
On 2016/11/08 04:49:11, dsansome wrote:
> 4-space indentation here

Done.

https://codereview.chromium.org/2484593002/diff/120001/masters/master.chromiu...
masters/master.chromium/public_html/gantt_chart.js:69: );
On 2016/11/08 04:49:11, dsansome wrote:
> And put this on the line above

Done.

[dsansome, 2016-11-09 05:08:49]

https://codereview.chromium.org/2484593002/diff/140001/masters/master.chromiu...
File masters/master.chromium/public_html/gantt_chart.js (right):

https://codereview.chromium.org/2484593002/diff/140001/masters/master.chromiu...
masters/master.chromium/public_html/gantt_chart.js:71:
document.getElementById('chart_div'));
You missed the indentation here.

[philwright, 2016-11-09 05:41:21]

https://codereview.chromium.org/2484593002/diff/140001/masters/master.chromiu...
File masters/master.chromium/public_html/gantt_chart.js (right):

https://codereview.chromium.org/2484593002/diff/140001/masters/master.chromiu...
masters/master.chromium/public_html/gantt_chart.js:71:
document.getElementById('chart_div'));
On 2016/11/09 05:08:49, dsansome wrote:
> You missed the indentation here.

Done.

[philwright, 2016-11-09 05:44:42]

The CQ bit was checked by philwright@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-11-09 05:44:47]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-09 05:54:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-09 05:54:18]

Dry run: This issue passed the CQ dry run.

[philwright, 2016-11-09 05:58:32]

The CQ bit was checked by philwright@google.com

[philwright, 2016-11-09 05:58:32]

The patchset sent to the CQ was uploaded after l-g-t-m from
dsansome@chromium.org
Link to the patchset: https://codereview.chromium.org/2484593002/#ps240001
(title: "rebased")

[commit-bot: I haz the power, 2016-11-09 05:58:38]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-09 06:09:00]

Description was changed from

==========
Adding gantt chart to Build Status page on Buildbot Masters

Screenshots:
Initial page load - https://screenshot.googleplex.com/NJjHFTcXU9T.png
After button clicked - https://screenshot.googleplex.com/QHn0DLd0Jv9.png

BUG=663245
TEST=local
 - ran local buildbot, generated screenshots
==========

to

==========
Adding gantt chart to Build Status page on Buildbot Masters

Screenshots:
Initial page load - https://screenshot.googleplex.com/NJjHFTcXU9T.png
After button clicked - https://screenshot.googleplex.com/QHn0DLd0Jv9.png

BUG=663245
TEST=local
 - ran local buildbot, generated screenshots

Committed:
https://chromium.googlesource.com/chromium/tools/build/+/f128e42562e0a67c1113...
==========

[commit-bot: I haz the power, 2016-11-09 06:09:01]

Committed patchset #13 (id:240001) as
https://chromium.googlesource.com/chromium/tools/build/+/f128e42562e0a67c1113...

[Michael Achenbach, 2016-11-09 10:52:01]

A revert of this CL (patchset #13 id:240001) has been created in
https://codereview.chromium.org/2489813002/ by machenbach@chromium.org.

The reason for reverting is: This fails on all masters the moment we sync. Now,
master1 was synced after a restart of chromium.fyi and this change became life
on all masters of master1. Log says:
	  File "../master.chromium/templates/build.html", line 12, in template
	    {'name': '{{ s.getName()|escapejs }}',
	jinja2.exceptions.TemplateAssertionError: no filter named 'escapejs'
.