code review 132650043: dashboard/builder: finish the previous -report=false change

dashboard/builder: finish the previous -report=false change

We weren't using the *report flag enough.

[bradfitz, 2014-09-05 20:15:32]

Hello proppy@google.com (cc: golang-codereviews@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go.tools

[proppy, 2014-09-05 21:31:35]

https://codereview.appspot.com/132650043/diff/40001/dashboard/builder/http.go
File dashboard/builder/http.go (right):

https://codereview.appspot.com/132650043/diff/40001/dashboard/builder/http.go...
dashboard/builder/http.go:109: if err = dash("GET", "todo", args, nil, &resp);
err != nil {
will that codepath (and other "GET" below) fail if there is no key?

https://codereview.appspot.com/132650043/diff/40001/dashboard/builder/main.go
File dashboard/builder/main.go (right):

https://codereview.appspot.com/132650043/diff/40001/dashboard/builder/main.go...
dashboard/builder/main.go:632: log.Printf("-report is false; disabling
commitWatcher")
Maybe you can move the if around the go commitWatcher() and also disable the
continuous build in main()?

[bradfitz, 2014-09-05 21:38:35]

https://codereview.appspot.com/132650043/diff/40001/dashboard/builder/http.go
File dashboard/builder/http.go (right):

https://codereview.appspot.com/132650043/diff/40001/dashboard/builder/http.go...
dashboard/builder/http.go:109: if err = dash("GET", "todo", args, nil, &resp);
err != nil {
On 2014/09/05 21:31:35, proppy wrote:
> will that codepath (and other "GET" below) fail if there is no key?

No.

I touched all the paths that send a key.

https://codereview.appspot.com/132650043/diff/40001/dashboard/builder/main.go
File dashboard/builder/main.go (right):

https://codereview.appspot.com/132650043/diff/40001/dashboard/builder/main.go...
dashboard/builder/main.go:632: log.Printf("-report is false; disabling
commitWatcher")
On 2014/09/05 21:31:35, proppy wrote:
> Maybe you can move the if around the go commitWatcher() and also disable the
> continuous build in main()?

I was just following the pattern to make this minimally invasive.

[proppy, 2014-09-05 21:47:09]

On 2014/09/05 21:38:35, bradfitz wrote:
> https://codereview.appspot.com/132650043/diff/40001/dashboard/builder/http.go
> File dashboard/builder/http.go (right):
> 
>
https://codereview.appspot.com/132650043/diff/40001/dashboard/builder/http.go...
> dashboard/builder/http.go:109: if err = dash("GET", "todo", args, nil, &resp);
> err != nil {
> On 2014/09/05 21:31:35, proppy wrote:
> > will that codepath (and other "GET" below) fail if there is no key?
> 
> No.
> 
> I touched all the paths that send a key.
> 
> https://codereview.appspot.com/132650043/diff/40001/dashboard/builder/main.go
> File dashboard/builder/main.go (right):
> 
>
https://codereview.appspot.com/132650043/diff/40001/dashboard/builder/main.go...
> dashboard/builder/main.go:632: log.Printf("-report is false; disabling
> commitWatcher")
> On 2014/09/05 21:31:35, proppy wrote:
> > Maybe you can move the if around the go commitWatcher() and also disable the
> > continuous build in main()?
> 
> I was just following the pattern to make this minimally invasive.

LGTM then :)

[bradfitz, 2014-09-05 21:48:15]

*** Submitted as
https://code.google.com/p/go/source/detail?r=20c1f7598a57&repo=tools ***

dashboard/builder: finish the previous -report=false change

We weren't using the *report flag enough.

LGTM=proppy
R=proppy
CC=golang-codereviews
https://codereview.appspot.com/132650043