Milo: Console view prototype

appengine/cmd/milo/git/gerrit.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 git
+
+import (
+        "encoding/json"
+        "fmt"
+        "strings"
+
+        "github.com/luci/luci-go/appengine/cmd/milo/resp"
+        "github.com/luci/luci-go/common/transport"
+        "golang.org/x/net/context"
+)
+
+type Repo struct {
+        Server string
+        Branch string
+}
+
+type GerritCommit struct {

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

this is Gitiles, not Gerrit

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

Done.

+        Log []struct {
+                Commit  string   `json:"commit"`
+                Tree    string   `json:"tree"`
+                Parents []string `json:"parents"`
+                Author  struct {

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

consider defining a type so you don't copy-paste type definitions inline

an alternative is to not parse Committer because you don't use it

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

Done.

+                        Name  string `json:"name"`
+                        Email string `json:"email"`
+                        Time  string `json:"time"`
+                } `json:"author"`
+                Committer struct {
+                        Name  string `json:"name"`
+                        Email string `json:"email"`
+                        Time  string `json:"time"`
+                } `json:"committer"`
+                Message string `json:"message"`
+        } `json:"log"`
+        Next string `json:"next"`
+}
+
+func GetCommits(c context.Context, repo, treeish string) ([]resp.Commit, error) {

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

repo looks like an URL (chromium.googlesource.com/chromium/src), but it is not
because does not have scheme. Consider renaming to repoURL and verify that it
starts with https://

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

Done.

+        URL := fmt.Sprintf("https://%s/+log/%s?format=JSON", repo, treeish)

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

add /a/ in the beginning, so you get a bigger quota
also make sure to whitelist your app in gitiles internal config (ask me how)

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

Done. App is already whitelisted.

+        client := transport.GetClient(c)

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

this needs to be rebased

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

...?

+        r, err := client.Get(URL)
+        if err != nil {
+                return nil, err
+        }
+        if r.StatusCode != 200 {
+                return nil, fmt.Errorf("Failed to fetch %s, status code %d", URL, r.StatusCode)
+        }
+        defer r.Body.Close()
+        trash := make([]byte, 4)

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

at least add a comment what is the prefix

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

Done.

+        r.Body.Read(trash) // Read the jsonp header
+        d := json.NewDecoder(r.Body)

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

inline d

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

Done.

+        cms := GerritCommit{}

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

what cms stands for?

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

commits

+        if err := d.Decode(&cms); err != nil {
+                return nil, err
+        }
+        // TODO(hinoka): Follow paging
+
+        // Move things into our own datastructure.
+        result := make([]resp.Commit, len(cms.Log))
+        for i, log := range cms.Log {
+                result[i] = resp.Commit{
+                        AuthorName:  log.Author.Name,
+                        AuthorEmail: log.Author.Email,
+                        Repo:        URL,

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

URL is not a repo URL
I think you meant repoURL

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

Done.

+                        Revision:    log.Commit,
+                        Description: log.Message,
+                        Title:       strings.SplitN(log.Message, "\n", 2)[0],
+                        // TODO(hinoka): The other stuff
+                }
+        }
+        return result, nil
+}