misc/dashboard/codereview: recognize "NOT LGTM".

A "NOT LGTM" overrules a previous "LGTM" by the same person, and vice versa.
"NOT LGTM"s are shown in the same location as LGTMs, colored red.

R=rsc
CC=golang-dev
https://golang.org/cl/6453062

diff --git a/misc/dashboard/codereview/dashboard/cl.go b/misc/dashboard/codereview/dashboard/cl.go
index 4201946832913a831bd4eea4b909e67397afd4f6..1b05e8ffdd81721eebbe5302303af53c94d921c8 100644 (file)
--- a/misc/dashboard/codereview/dashboard/cl.go
+++ b/misc/dashboard/codereview/dashboard/cl.go
@@ -49,6 +49,7 @@ type CL struct {
        Description []byte `datastore:",noindex"`
        FirstLine   string `datastore:",noindex"`
        LGTMs       []string
+       NotLGTMs    []string
 
        // Mail information.
        Subject       string   `datastore:",noindex"`
@@ -78,9 +79,9 @@ func (cl *CL) FirstLineHTML() template.HTML {
        return template.HTML(s)
 }
 
-func (cl *CL) LGTMHTML() template.HTML {
-       x := make([]string, len(cl.LGTMs))
-       for i, s := range cl.LGTMs {
+func formatEmails(e []string) template.HTML {
+       x := make([]string, len(e))
+       for i, s := range e {
                s = template.HTMLEscapeString(s)
                if !strings.Contains(s, "@") {
                        s = "<b>" + s + "</b>"
@@ -91,6 +92,14 @@ func (cl *CL) LGTMHTML() template.HTML {
        return template.HTML(strings.Join(x, ", "))
 }
 
+func (cl *CL) LGTMHTML() template.HTML {
+       return formatEmails(cl.LGTMs)
+}
+
+func (cl *CL) NotLGTMHTML() template.HTML {
+       return formatEmails(cl.NotLGTMs)
+}
+
 func (cl *CL) ModifiedAgo() string {
        // Just the first non-zero unit.
        units := [...]struct {
@@ -326,6 +335,7 @@ func updateCL(c appengine.Context, n string) error {
                cl.FirstLine = cl.FirstLine[:i]
        }
        lgtm := make(map[string]bool)
+       notLGTM := make(map[string]bool)
        rcpt := make(map[string]bool)
        for _, msg := range apiResp.Messages {
                s, rev := msg.Sender, false
@@ -343,6 +353,11 @@ func updateCL(c appengine.Context, n string) error {
 
                if msg.Approval {
                        lgtm[s] = true
+                       delete(notLGTM, s) // "LGTM" overrules previous "NOT LGTM"
+               }
+               if strings.Contains(msg.Text, "NOT LGTM") {
+                       notLGTM[s] = true
+                       delete(lgtm, s) // "NOT LGTM" overrules previous "LGTM"
                }
 
                for _, r := range msg.Recipients {
@@ -352,10 +367,14 @@ func updateCL(c appengine.Context, n string) error {
        for l := range lgtm {
                cl.LGTMs = append(cl.LGTMs, l)
        }
+       for l := range notLGTM {
+               cl.NotLGTMs = append(cl.NotLGTMs, l)
+       }
        for r := range rcpt {
                cl.Recipients = append(cl.Recipients, r)
        }
        sort.Strings(cl.LGTMs)
+       sort.Strings(cl.NotLGTMs)
        sort.Strings(cl.Recipients)
 
        err = datastore.RunInTransaction(c, func(c appengine.Context) error {

diff --git a/misc/dashboard/codereview/dashboard/front.go b/misc/dashboard/codereview/dashboard/front.go
index 791a1000c23586daae35adb5e3dce6ff59fe1a69..052f50d367a3e2a94d10495a8391d7a034f6f9f8 100644 (file)
--- a/misc/dashboard/codereview/dashboard/front.go
+++ b/misc/dashboard/codereview/dashboard/front.go
@@ -245,6 +245,7 @@ var frontPage = template.Must(template.New("front").Funcs(template.FuncMap{
     <td>
       <a href="http://codereview.appspot.com/{{.Number}}/" title="{{ printf "%s" .Description}}">{{.Number}}: {{.FirstLineHTML}}</a>
       {{if and .LGTMs $tbl.Assignable}}<br /><span style="font-size: smaller;">LGTMs: {{.LGTMHTML}}{{end}}</span>
+      {{if and .NotLGTMs $tbl.Assignable}}<br /><span style="font-size: smaller; color: #f74545;">NOT LGTMs: {{.NotLGTMHTML}}{{end}}</span>
     </td>
     <td title="Last modified">{{.ModifiedAgo}}</td>
     {{if $.IsAdmin}}<td><a href="/update-cl?cl={{.Number}}" title="Update this CL">&#x27f3;</a></td>{{end}}