Move builderList creation to ct-failure-card

Move builderList creation to ct-failure-card

Another change needed for https://codereview.chromium.org/502293003/

I believe this also fixes a bug I introduced where builderList
was not updated when failures change :(

R=ojan@chromium.org
NOTRY=true

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=181594

[ojan, 2014-09-05 01:18:04]

lgtm

https://codereview.chromium.org/541123002/diff/20001/Tools/GardeningServer/ui...
File Tools/GardeningServer/ui/ct-failure-card.html (right):

https://codereview.chromium.org/541123002/diff/20001/Tools/GardeningServer/ui...
Tools/GardeningServer/ui/ct-failure-card.html:106: 'group.failures':
'_updateBuilderList',
Woah. Didn't realize you could do this.

https://codereview.chromium.org/541123002/diff/20001/Tools/GardeningServer/ui...
Tools/GardeningServer/ui/ct-failure-card.html:127: this._builderList = new
CTBuilderList(this.group.failures);
I think we still have a bug where we won't update the builder list if it changes
within a given CTFailure, but that's not new in this patch. But, maybe add a
FIXME? This won't really show up until jyasskin lands his patch making us do
incremental updates (today or tomorrow).

[cbiesinger, 2014-09-06 00:25:56]

https://codereview.chromium.org/541123002/diff/20001/Tools/GardeningServer/ui...
File Tools/GardeningServer/ui/ct-failure-card.html (right):

https://codereview.chromium.org/541123002/diff/20001/Tools/GardeningServer/ui...
Tools/GardeningServer/ui/ct-failure-card.html:106: 'group.failures':
'_updateBuilderList',
On 2014/09/05 01:18:04, ojan-only-code-yellow-reviews wrote:
> Woah. Didn't realize you could do this.

Yeah, also $.foo.bar for polymer elements in your child tree. See
http://www.polymer-project.org/docs/polymer/polymer.html#change-watchers

https://codereview.chromium.org/541123002/diff/20001/Tools/GardeningServer/ui...
Tools/GardeningServer/ui/ct-failure-card.html:127: this._builderList = new
CTBuilderList(this.group.failures);
On 2014/09/05 01:18:04, ojan-only-code-yellow-reviews wrote:
> I think we still have a bug where we won't update the builder list if it
changes
> within a given CTFailure, but that's not new in this patch. But, maybe add a
> FIXME? This won't really show up until jyasskin lands his patch making us do
> incremental updates (today or tomorrow).

Oy, that's true, but I have no idea how to fix that? I'll add a FIXME.

[cbiesinger, 2014-09-06 00:27:27]

The CQ bit was checked by cbiesinger@chromium.org

[commit-bot: I haz the power, 2014-09-06 00:29:02]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/cbiesinger@chromium.org/541123002/40001

[commit-bot: I haz the power, 2014-09-06 00:29:12]

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

[commit-bot: I haz the power, 2014-09-06 00:29:12]

Failed to apply patch for Tools/GardeningServer/ui/ct-failure-card.html:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file Tools/GardeningServer/ui/ct-failure-card.html
  Hunk #1 FAILED at 65.
  Hunk #2 succeeded at 95 (offset -2 lines).
  Hunk #3 succeeded at 121 (offset -2 lines).
  1 out of 3 hunks FAILED -- saving rejects to file
Tools/GardeningServer/ui/ct-failure-card.html.rej

Patch:       Tools/GardeningServer/ui/ct-failure-card.html
Index: Tools/GardeningServer/ui/ct-failure-card.html
diff --git a/Tools/GardeningServer/ui/ct-failure-card.html
b/Tools/GardeningServer/ui/ct-failure-card.html
index
84dca8b99bc095496a5a6785ab3384426da56358..ede85c30fbfc67a80049939a141f6fb34cc14876
100644
--- a/Tools/GardeningServer/ui/ct-failure-card.html
+++ b/Tools/GardeningServer/ui/ct-failure-card.html
@@ -65,7 +65,7 @@ found in the LICENSE file.
         opacity: 0.5;
       }
     </style>
-    <ct-builder-grid failures="{{ group.failures }}"></ct-builder-grid>
+    <ct-builder-grid builderList="{{ _builderList }}"></ct-builder-grid>
     <div id="failure" class="{{ { snoozed: group.isSnoozed } | tokenList }}">
       <ct-test-list tests="{{ group.failures }}" tree="{{ tree
}}"></ct-test-list>
       <ct-commit-list commitList="{{ group.commitList }}"></ct-commit-list>
@@ -97,11 +97,13 @@ found in the LICENSE file.
     Polymer({
       group: null,
       commitLog: null,
+      builderList: null,
       tree: '',
 
       observe: {
         group: '_updateCommitList',
-        commitLog: '_updateCommitList'
+        commitLog: '_updateCommitList',
+        'group.failures': '_updateBuilderList',
       },
 
       examine: function() {
@@ -121,6 +123,11 @@ found in the LICENSE file.
           this.group.commitList.update(this.commitLog);
       },
 
+      _updateBuilderList: function() {
+        // FIXME: We need to also observe changes within a given CTFailure.
+        this._builderList = new CTBuilderList(this.group.failures);
+      },
+
       linkBug: function() {
         this.$.bug.value = this.group.bug;
         this.$.bugDialog.toggle();

[cbiesinger, 2014-09-06 00:55:44]

That merge seems surprisingly hard :( Next week, I guess...

[cbiesinger, 2014-09-09 01:42:41]

Hi Ojan - I finally rebased this, do you want to review it again? It does look a
bit different now, with the new trooper stuff. Feel free to put in the cq.

[ojan, 2014-09-09 02:09:29]

The CQ bit was checked by ojan@chromium.org

[ojan, 2014-09-09 02:09:29]

lgtm

[commit-bot: I haz the power, 2014-09-09 02:09:50]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/cbiesinger@chromium.org/541123002/60001

[commit-bot: I haz the power, 2014-09-09 02:10:37]

Committed patchset #4 (id:60001) as 181594