Milo buildbot: Add in the rest of the blame/commits information.

milo/appengine/buildbot/structs.go

 // Copyright 2016 The LUCI Authors. All rights reserved.
 // Use of this source code is governed under the Apache License, Version 2.0
 // that can be found in the LICENSE file.
 
 package buildbot
 
 import (
         "bytes"
         "compress/gzip"
         "encoding/json"
@@ skipping 316 matching lines @@
         Project    string          `json:"project"`
         Properties [][]interface{} `json:"properties"`
         Repository string          `json:"repository"`
         Rev        string          `json:"rev"`
         Revision   string          `json:"revision"`
         Revlink    string          `json:"revlink"`
         When       int             `json:"when"`
         Who        string          `json:"who"`
 }
 
+func (bc *buildbotChange) GetFiles() []string {
+        files := make([]string, len(bc.Files))
+        for i, f := range bc.Files {
+                // Buildbot stores files both as a string, or as a dict with a single entry
+                // named "name".  It doesn't matter to us what the type is, but we need
+                // to reflect on the type anyways.
+                if fn, ok := f.(string); ok {

[nodir, 2017/04/19 18:37:48]

i think a type-switch would be cleaner here

[hinoka, 2017/04/20 22:09:43]

Done.

+                        files[i] = fn
+                } else if fn, ok := f.(map[string]interface{}); ok {
+                        if name, ok := fn["name"]; ok {
+                                files[i] = fmt.Sprintf("%s", name)
+                        } else {
+                                files[i] = fmt.Sprintf("%#v", f)

[nodir, 2017/04/19 18:37:48]

i think if a user sees such as string, they would perceive it as a sloppy bug.
WDYT about logging an error/warning and not omitting?

[nodir, 2017/04/20 03:24:08]

I meant omitting

[hinoka, 2017/04/20 22:09:43]

Done.  It just omits it without logging because I don't want to pass the context
into this function.  The context has no other use in here and it would just
otherwise complicate things.

+                        }
+                } else {
+                        files[i] = fmt.Sprintf("%#v", f)
+                }
+        }
+        return files
+}
+
 // buildbotSlave describes a slave on a master from a master json, and also includes the
 // full builds of any currently running builds.
 type buildbotSlave struct {
         // RecentBuilds is a map of builder name to a list of recent finished build
         // numbers on that builder.
         RecentBuilds  map[string][]int `json:"builders"`
         Connected     bool             `json:"connected"`
         Host          string           `json:"host"`
         Name          string           `json:"name"`
         Runningbuilds []*buildbotBuild `json:"runningBuilds"`
@@ skipping 67 matching lines @@
                         PendingBuilds   int    `json:"pending_builds"`
                         State           string `json:"state"`
                         TotalSlaves     int    `json:"total_slaves"`
                 } `json:"builders"`
                 ServerUptime float64 `json:"server_uptime"`
         } `json:"varz"`
 
         // This is injected by the pubsub publisher on the buildbot side.
         Name string `json:"name"`
 }