code review 164960043: go.tools/dashboard/app: update commit in transaction on...

go.tools/dashboard/app: update commit in transaction on perf regression

The sendPerfFailMail function populated a dummy commit value and
then calls commonNotify, which then updated and stored that dummy
commit, hosing the original commit entity.

[adg, 2014-10-27 21:48:24]

Hello rsc, bradfitz, dvyukov (cc: golang-codereviews@googlegroups.com),

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

[rsc, 2014-10-28 00:10:04]

LGTM

[adg, 2014-10-28 00:25:09]

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

go.tools/dashboard/app: update commit in transaction on perf regression

The sendPerfFailMail function populated a dummy commit value and
then calls commonNotify, which then updated and stored that dummy
commit, hosing the original commit entity.

LGTM=rsc
R=rsc, bradfitz, dvyukov
CC=golang-codereviews
https://codereview.appspot.com/164960043

[dvyukov, 2014-10-28 12:17:39]

Aha! Here it is.
LGTM

That code runs in a transaction already. I guess nested transactions just gets
flattened.

[adg, 2014-10-29 00:54:00]

Actually that's probably an error on my part. I'll send a fix removing the
transaction part.
The real fix here is fetching the commit before storing it.

On Tue Oct 28 2014 at 11:17:42 PM <dvyukov@google.com> wrote:

> Aha! Here it is.
> LGTM
>
> That code runs in a transaction already. I guess nested transactions
> just gets flattened.
>
> https://codereview.appspot.com/164960043/
>