Add last update time to Sheriff-O-Matic.

Add last update time to Sheriff-O-Matic.

This uses the "date" given in the http://auto-sheriff.appspot.com/data page to
show how recent the failure information is.

I intend to add some styling when the data is more than 5 minutes old, but I'm
pretty new to Polymer and want to make sure this is on track first.

BUG=399712
NOTRY=true

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

[ojan, 2014-08-08 06:05:13]

Thanks for helping out!

https://codereview.chromium.org/453543002/diff/1/Tools/GardeningServer/ui/ct-...
File Tools/GardeningServer/ui/ct-failure-analyzer.html (right):

https://codereview.chromium.org/453543002/diff/1/Tools/GardeningServer/ui/ct-...
Tools/GardeningServer/ui/ct-failure-analyzer.html:16: date: null,
Lets call this lastUpdateDate to be more explicit.

https://codereview.chromium.org/453543002/diff/1/Tools/GardeningServer/ui/ct-...
Tools/GardeningServer/ui/ct-failure-analyzer.html:39: this.date = data.date;
Lets store an actual Date object instead of a timestamp. Then you'll be able to
use Date methods in ct-last-updated.

https://codereview.chromium.org/453543002/diff/1/Tools/GardeningServer/ui/ct-...
File Tools/GardeningServer/ui/ct-last-updated.html (right):

https://codereview.chromium.org/453543002/diff/1/Tools/GardeningServer/ui/ct-...
Tools/GardeningServer/ui/ct-last-updated.html:7: <polymer-element
name="ct-last-updated" attributes="date">
Please add a test for this. It's confusing because we're transitioning from one
test harness to another. See README-mocha and write the tests using the
mocha+chai harness, not the qunit one.

https://codereview.chromium.org/453543002/diff/1/Tools/GardeningServer/ui/ct-...
Tools/GardeningServer/ui/ct-last-updated.html:11: display: flex;
I don't think you need this element to be a flexbox.

https://codereview.chromium.org/453543002/diff/1/Tools/GardeningServer/ui/ct-...
Tools/GardeningServer/ui/ct-last-updated.html:18: white-space: nowrap;
It's tricky deciding where to put which CSS styling. This sort of styling is the
sort of thing the outer page may want to control (e.g. if this component is
included in two different places, one may want to clip and one may not).

In the future patch in which you add colors to show when the update time gets
too long, I think that would be appropriate styling to include here inside the
component because it's tied to logic in the component itself instead of how the
outer page uses the component.

https://codereview.chromium.org/453543002/diff/1/Tools/GardeningServer/ui/ct-...
Tools/GardeningServer/ui/ct-last-updated.html:22: <div class="message">Updated
{{ delta_min }} min ago @ {{ date_hours }}:{{ date_minutes }}.</div>
I don't think this wrapper div is needed.

https://codereview.chromium.org/453543002/diff/1/Tools/GardeningServer/ui/ct-...
Tools/GardeningServer/ui/ct-last-updated.html:36: update: function() {
You don't need the update function. Better is to let Polymer's data binding do
this work for you.

Updated {{ date | _deltaMinutes }} min ago @ {{ date | _hours }}:{{ date :
_minutes }}.

Then on your polymer element define:
_deltaMinutes: function(date) {
  return date.relative(...);
},
_hours: function (date) {
...

Then you don't need to do any manual updating. When the date object changes,
polymer will update all these things for you.

https://codereview.chromium.org/453543002/diff/1/Tools/GardeningServer/ui/ct-...
Tools/GardeningServer/ui/ct-last-updated.html:39: this.date_minutes = new
Date(this.date * 1000).getMinutes();
sugarjs, which we include, has nice methods to get these things. Use
http://sugarjs.com/api/Date/unitsAgo for the hours/minutes and
http://sugarjs.com/api/Date/relative for the deltaMinutes.

https://codereview.chromium.org/453543002/diff/1/Tools/GardeningServer/ui/ct-...
File Tools/GardeningServer/ui/ct-unexpected-failures.html (right):

https://codereview.chromium.org/453543002/diff/1/Tools/GardeningServer/ui/ct-...
Tools/GardeningServer/ui/ct-unexpected-failures.html:41: <ct-last-updated
id="last_updated" date="{{ date }}"></ct-last-updated>
I'd like to conserve this screen real estate as much as possible. I was thinking
of putting the update message in the black bar at the top, to the left of the
select dropdown. That does mean you'll need to pass the date up into
ct-sheriff-o-matic. I'm not sure I'll like it up there, but lets try that first
and we can change it if it's too ugly.

[teravest, 2014-08-08 22:21:28]

https://codereview.chromium.org/453543002/diff/1/Tools/GardeningServer/ui/ct-...
File Tools/GardeningServer/ui/ct-failure-analyzer.html (right):

https://codereview.chromium.org/453543002/diff/1/Tools/GardeningServer/ui/ct-...
Tools/GardeningServer/ui/ct-failure-analyzer.html:16: date: null,
On 2014/08/08 06:05:12, ojan-only-code-yellow-reviews wrote:
> Lets call this lastUpdateDate to be more explicit.

Done.

https://codereview.chromium.org/453543002/diff/1/Tools/GardeningServer/ui/ct-...
Tools/GardeningServer/ui/ct-failure-analyzer.html:39: this.date = data.date;
On 2014/08/08 06:05:12, ojan-only-code-yellow-reviews wrote:
> Lets store an actual Date object instead of a timestamp. Then you'll be able
to
> use Date methods in ct-last-updated.

Done.

https://codereview.chromium.org/453543002/diff/1/Tools/GardeningServer/ui/ct-...
File Tools/GardeningServer/ui/ct-last-updated.html (right):

https://codereview.chromium.org/453543002/diff/1/Tools/GardeningServer/ui/ct-...
Tools/GardeningServer/ui/ct-last-updated.html:7: <polymer-element
name="ct-last-updated" attributes="date">
On 2014/08/08 06:05:13, ojan-only-code-yellow-reviews wrote:
> Please add a test for this. It's confusing because we're transitioning from
one
> test harness to another. See README-mocha and write the tests using the
> mocha+chai harness, not the qunit one.

Done.

https://codereview.chromium.org/453543002/diff/1/Tools/GardeningServer/ui/ct-...
Tools/GardeningServer/ui/ct-last-updated.html:11: display: flex;
On 2014/08/08 06:05:13, ojan-only-code-yellow-reviews wrote:
> I don't think you need this element to be a flexbox.

Removed.

https://codereview.chromium.org/453543002/diff/1/Tools/GardeningServer/ui/ct-...
Tools/GardeningServer/ui/ct-last-updated.html:18: white-space: nowrap;
On 2014/08/08 06:05:12, ojan-only-code-yellow-reviews wrote:
> It's tricky deciding where to put which CSS styling. This sort of styling is
the
> sort of thing the outer page may want to control (e.g. if this component is
> included in two different places, one may want to clip and one may not).
> 
> In the future patch in which you add colors to show when the update time gets
> too long, I think that would be appropriate styling to include here inside the
> component because it's tied to logic in the component itself instead of how
the
> outer page uses the component.

I've removed the styling here.

https://codereview.chromium.org/453543002/diff/1/Tools/GardeningServer/ui/ct-...
Tools/GardeningServer/ui/ct-last-updated.html:22: <div class="message">Updated
{{ delta_min }} min ago @ {{ date_hours }}:{{ date_minutes }}.</div>
On 2014/08/08 06:05:12, ojan-only-code-yellow-reviews wrote:
> I don't think this wrapper div is needed.

Removed.

https://codereview.chromium.org/453543002/diff/1/Tools/GardeningServer/ui/ct-...
Tools/GardeningServer/ui/ct-last-updated.html:36: update: function() {
On 2014/08/08 06:05:13, ojan-only-code-yellow-reviews wrote:
> You don't need the update function. Better is to let Polymer's data binding do
> this work for you.
> 
> Updated {{ date | _deltaMinutes }} min ago @ {{ date | _hours }}:{{ date :
> _minutes }}.
> 
> Then on your polymer element define:
> _deltaMinutes: function(date) {
>   return date.relative(...);
> },
> _hours: function (date) {
> ...
> 
> Then you don't need to do any manual updating. When the date object changes,
> polymer will update all these things for you.

That's much better, thanks!

https://codereview.chromium.org/453543002/diff/1/Tools/GardeningServer/ui/ct-...
Tools/GardeningServer/ui/ct-last-updated.html:39: this.date_minutes = new
Date(this.date * 1000).getMinutes();
On 2014/08/08 06:05:12, ojan-only-code-yellow-reviews wrote:
> sugarjs, which we include, has nice methods to get these things. Use
> http://sugarjs.com/api/Date/unitsAgo for the hours/minutes and
> http://sugarjs.com/api/Date/relative for the deltaMinutes.

Excellent. Done.

https://codereview.chromium.org/453543002/diff/1/Tools/GardeningServer/ui/ct-...
File Tools/GardeningServer/ui/ct-unexpected-failures.html (right):

https://codereview.chromium.org/453543002/diff/1/Tools/GardeningServer/ui/ct-...
Tools/GardeningServer/ui/ct-unexpected-failures.html:41: <ct-last-updated
id="last_updated" date="{{ date }}"></ct-last-updated>
On 2014/08/08 06:05:13, ojan-only-code-yellow-reviews wrote:
> I'd like to conserve this screen real estate as much as possible. I was
thinking
> of putting the update message in the black bar at the top, to the left of the
> select dropdown. That does mean you'll need to pass the date up into
> ct-sheriff-o-matic. I'm not sure I'll like it up there, but lets try that
first
> and we can change it if it's too ugly.

Sounds good. I've moved this into the top bar and aligned it to the right over
with the select dropdown. Polymer is pretty slick; moving that around was easier
than I thought.

[ojan, 2014-08-09 03:54:21]

lgtm!

FYI, you can NOTRY=true these patches since they don't run on the CQ.

[commit-bot: I haz the power, 2014-08-09 03:55:32]

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

[commit-bot: I haz the power, 2014-08-09 03:55:41]

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

[commit-bot: I haz the power, 2014-08-09 03:55:42]

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

Patch:       Tools/GardeningServer/ui/ct-failure-analyzer.html
Index: Tools/GardeningServer/ui/ct-failure-analyzer.html
diff --git a/Tools/GardeningServer/ui/ct-failure-analyzer.html
b/Tools/GardeningServer/ui/ct-failure-analyzer.html
index
f31bc4af8cf6c55f313cf59c22008843cefd860b..f01909e4344eecdce26baf113a44114ef598f03f
100644
--- a/Tools/GardeningServer/ui/ct-failure-analyzer.html
+++ b/Tools/GardeningServer/ui/ct-failure-analyzer.html
@@ -6,13 +6,14 @@ found in the LICENSE file.
 
 <link rel="import" href="../model/ct-failure.html">
 
-<polymer-element name="ct-failure-analyzer" attributes="failures
builderLatestRevisions">
+<polymer-element name="ct-failure-analyzer" attributes="failures
builderLatestRevisions lastUpdateDate">
   <script>
     // FIXME: Don't use a polymer component for this. Instead use a Failures
model
     // object that knows how to do the XHR and process the data appropriately.
     Polymer({
       builderLatestRevisions: null,
       failures: null,
+      lastUpdateDate: null,
 
       // FIXME: Get this from
https://chromium.googlesource.com/chromium/tools/build/+/master/scripts/slave....
       _trees: {
@@ -35,6 +36,7 @@ found in the LICENSE file.
         net.json('http://auto-sheriff.appspot.com/data').then(function(data) {
           // FIXME: Don't special-case the blink master.
           this.builderLatestRevisions =
data.latest_revisions['chromium.webkit'];
+          this.lastUpdateDate = new Date(data.date * 1000);
           // FIXME: Make this a model class intead of a dumb object.
           this.failures = {};
           data.range_groups.forEach(function(group) {

[teravest, 2014-08-11 14:31:18]

The CQ bit was checked by teravest@chromium.org

[commit-bot: I haz the power, 2014-08-11 14:31:44]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/teravest@chromium.org/453543002/80001

[commit-bot: I haz the power, 2014-08-11 14:33:10]

Change committed as 179953