Milo: Console view prototype

appengine/cmd/milo/console/console.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 console
+
+import (
+        "fmt"
+        "strings"
+
+        "github.com/luci/luci-go/appengine/cmd/milo/buildbot"
+        "github.com/luci/luci-go/appengine/cmd/milo/git"
+        "github.com/luci/luci-go/appengine/cmd/milo/resp"
+        "github.com/luci/luci-go/common/clock"
+        log "github.com/luci/luci-go/common/logging"
+        "golang.org/x/net/context"
+)
+
+func resolveBuilders(c context.Context, builderSpec []string) []resp.BuilderRef {
+        result := []resp.BuilderRef{}
+        for _, spec := range builderSpec {
+                j := strings.SplitN(spec, "/", 2)
+                result = append(result, resp.BuilderRef{
+                        Module: j[0],
+                        Name:   j[1],
+                })
+        }
+        return result
+}
+
+// Returns results of build[commit_index][builder_index]
+func GetConsoleBuilds(
+        c context.Context, module string,
+        builders []resp.BuilderRef, commits []string) (
+        [][]*resp.ConsoleBuild, error) {
+
+        switch module {
+        case "buildbot":
+                return buildbot.GetConsoleBuilds(c, builders, commits)
+        //case "buildbucket":
+        //      return buildbucket.getConsoleBuilds(c, builders, commits)
+        default:
+                panic(fmt.Errorf("Unrecognized module %s", module))

[nodir, 2016/08/03 20:26:17]

a panic kinda implies that the caller really knows what module they want. If so,
I am not sure what's the value of this function if the caller can just call
buildbot.GetConsoleBuilds or buildbucket.GetConsoleBuilds

[hinoka, 2016/08/03 21:55:39]

The idea of this function is to map strings to linked modules.  The caller knows
the name of the module, but unlike python it cannot resolve the name of a module
to an actual go package.  This function is supposed to serve as the map.

+        }
+}
+
+func common(current *resp.BuilderGroup, path string) int {
+        result := 0
+        g := strings.Split(current.FullPath, "|")
+        for i, s := range strings.Split(path, "|") {
+                if i < len(g) && g[i] == s {
+                        result++
+                } else {
+                        break
+                }
+        }
+        return result
+}
+
+func console(c context.Context, def *ConsoleDef) (
+        *resp.Console, error) {

[nodir, 2016/08/03 20:26:17]

i think it would fit prev line

[hinoka, 2016/08/03 21:55:40]

Done.

+        t1 := clock.Now(c)
+        // Lookup Commits.  For this hack, we're just gonna hardcode src.git
+        commits, err := git.GetCommits(c, def.Repository, def.Branch)
+        if err != nil {
+                return nil, err
+        }
+        t2 := clock.Now(c)

[nodir, 2016/08/03 20:26:17]

please give more meaning full names to t variables, e.g. tStarted,
tCommitsLoaded
you won't have to rename t variables when you insert a new t in between existing
t's

[hinoka, 2016/08/03 21:55:40]

Done.

+        log.Debugf(c, fmt.Sprintf("Gerrit took %f seconds.", t2.Sub(t1).Seconds()))

[nodir, 2016/08/03 20:26:17]

use native representation of durations:
took %s", r2.Sub(t1))

[nodir, 2016/08/03 20:26:17]

s/Gerrit/Loading commits/

[nodir, 2016/08/03 20:26:17]

log.Debugf accepts a format string, so no need for Sprintf

[hinoka, 2016/08/03 21:55:40]

Done.

+        commitNames := make([]string, len(commits))
+        for i, commit := range commits {
+                commitNames[i] = commit.Revision
+        }
+
+        // HACK(hinoka): This only supports buildbot....
+        cb, err := GetConsoleBuilds(c, "buildbot", def.Builders, commitNames)

[nodir, 2016/08/03 20:26:17]

time it?

[hinoka, 2016/08/03 21:55:40]

Done.

+        if err != nil {
+                return nil, err
+        }
+        ccb := make([]resp.CommitBuild, len(commits))
+        for i, commit := range commitNames {
+                // TODO(hinoka): Not like this
+                ccb[i].Commit = resp.Commit{Revision: commit}
+                ccb[i].Build = cb[i]
+        }
+
+        cs := &resp.Console{
+                Name:       def.Name,
+                Commit:     ccb,
+                BuilderRef: def.Builders,
+        }
+
+        return cs, nil
+}