Implement Mail service.

Implement Mail service.

R=dnj@chromium.org, estaab@chromium.org, martiniss@chromium.org, vadimsh@chromium.org
BUG=https://github.com/luci/gae/issues/20

Committed: https://github.com/luci/gae/commit/86e47afb5d2c2a20678a9003661bc9316539274b

[iannucci, 2015-12-15 02:52:30]

On 2015/12/15 at 02:51:43, iannucci wrote:
> 

Still needs tests for the memory version, but PTAL for overall interface (normal
and testing).

[M-A Ruel, 2015-12-15 13:37:23]

Thanks as I was discussing with Erik yesterday that creating a Go version of
ereporter2 would be very useful, and this requires email support.

[iannucci, 2015-12-15 20:03:20]

PTAL, added full test coverage and fixed bugs it found.

[iannucci, 2015-12-16 20:31:07]

On 2015/12/15 at 20:03:20, iannucci wrote:
> PTAL, added full test coverage and fixed bugs it found.

ping

[martiniss, 2015-12-16 20:32:16]

Who's lucky day is it?

[martiniss, 2015-12-16 20:37:44]

On 2015/12/16 at 20:32:16, martiniss wrote:
> Who's lucky day is it?

(picking a reviewer would probably get a faster response than sending to 4
people :) )

[iannucci, 2015-12-16 20:41:40]

On 2015/12/16 at 20:37:44, martiniss wrote:
> On 2015/12/16 at 20:32:16, martiniss wrote:
> > Who's lucky day is it?
> 
> (picking a reviewer would probably get a faster response than sending to 4
people :) )

erm... who has more time?

[martiniss, 2015-12-16 21:25:34]

On 2015/12/16 at 20:41:40, iannucci wrote:
> On 2015/12/16 at 20:37:44, martiniss wrote:
> > On 2015/12/16 at 20:32:16, martiniss wrote:
> > > Who's lucky day is it?
> > 
> > (picking a reviewer would probably get a faster response than sending to 4
people :) )
> 
> erm... who has more time?

Was waiting for someone else to volunteer.... I'll just do it.

[martiniss, 2015-12-16 21:54:15]

pretty good!

https://codereview.chromium.org/1525923002/diff/20001/impl/dummy/dummy_test.go
File impl/dummy/dummy_test.go (right):

https://codereview.chromium.org/1525923002/diff/20001/impl/dummy/dummy_test.g...
impl/dummy/dummy_test.go:86: Convey("Mail", func() {
Bad indentation?

https://codereview.chromium.org/1525923002/diff/20001/impl/memory/mail.go
File impl/memory/mail.go (right):

https://codereview.chromium.org/1525923002/diff/20001/impl/memory/mail.go#new...
impl/memory/mail.go:112: canonK := textproto.CanonicalMIMEHeaderKey(k)
Is this the same way the app engine people do it?

https://codereview.chromium.org/1525923002/diff/20001/impl/memory/mail.go#new...
impl/memory/mail.go:189: msgs := m.data.queue[:]
[:] means a new slice, but it's not a copy, so you need to re-copy. Correct?

https://codereview.chromium.org/1525923002/diff/20001/impl/memory/user_test.go
File impl/memory/user_test.go (right):

https://codereview.chromium.org/1525923002/diff/20001/impl/memory/user_test.g...
impl/memory/user_test.go:92: }, ShouldPanicLike, `mail: missing phrase`)
Ummm. This error message doesn't mean much to me. I guess this comes from the
SDK? :/

https://codereview.chromium.org/1525923002/diff/20001/service/mail/interface.go
File service/mail/interface.go (right):

https://codereview.chromium.org/1525923002/diff/20001/service/mail/interface....
service/mail/interface.go:10: SendToAdmins(msg *Message) error
Why is this a separate function? Also, do these need to be documented?

https://codereview.chromium.org/1525923002/diff/20001/service/mail/message.go
File service/mail/message.go (right):

https://codereview.chromium.org/1525923002/diff/20001/service/mail/message.go...
service/mail/message.go:1: // Copyright 2015 The Chromium Authors. All rights
reserved.
Couple simple tests for this? Just to get some basic coverage.

[iannucci, 2015-12-17 00:01:07]

https://codereview.chromium.org/1525923002/diff/20001/impl/dummy/dummy_test.go
File impl/dummy/dummy_test.go (right):

https://codereview.chromium.org/1525923002/diff/20001/impl/dummy/dummy_test.g...
impl/dummy/dummy_test.go:86: Convey("Mail", func() {
On 2015/12/16 at 21:54:15, martiniss wrote:
> Bad indentation?

Rietveld bug? Check the new patch, but it looks fine here :/

https://codereview.chromium.org/1525923002/diff/20001/impl/memory/mail.go
File impl/memory/mail.go (right):

https://codereview.chromium.org/1525923002/diff/20001/impl/memory/mail.go#new...
impl/memory/mail.go:112: canonK := textproto.CanonicalMIMEHeaderKey(k)
On 2015/12/16 at 21:54:15, martiniss wrote:
> Is this the same way the app engine people do it?

yes, it's the same way that all mime implementations (should) do it.

https://codereview.chromium.org/1525923002/diff/20001/impl/memory/mail.go#new...
impl/memory/mail.go:189: msgs := m.data.queue[:]
On 2015/12/16 at 21:54:15, martiniss wrote:
> [:] means a new slice, but it's not a copy, so you need to re-copy. Correct?

no, the queue is an replaceable slice of immutable data. I just want to capture
the current slice of data.

https://codereview.chromium.org/1525923002/diff/20001/impl/memory/user_test.go
File impl/memory/user_test.go (right):

https://codereview.chromium.org/1525923002/diff/20001/impl/memory/user_test.g...
impl/memory/user_test.go:92: }, ShouldPanicLike, `mail: missing phrase`)
On 2015/12/16 at 21:54:15, martiniss wrote:
> Ummm. This error message doesn't mean much to me. I guess this comes from the
SDK? :/

comes from "net/mail"

https://codereview.chromium.org/1525923002/diff/20001/service/mail/interface.go
File service/mail/interface.go (right):

https://codereview.chromium.org/1525923002/diff/20001/service/mail/interface....
service/mail/interface.go:10: SendToAdmins(msg *Message) error
On 2015/12/16 at 21:54:15, martiniss wrote:
> Why is this a separate function? Also, do these need to be documented?

These are the same as the functions in the SDK. I'll mention that in the
interface doc string, but I don't want to copy-paste documentation (that said,
the upstream docs are pretty bad too...)

https://codereview.chromium.org/1525923002/diff/20001/service/mail/message.go
File service/mail/message.go (right):

https://codereview.chromium.org/1525923002/diff/20001/service/mail/message.go...
service/mail/message.go:1: // Copyright 2015 The Chromium Authors. All rights
reserved.
On 2015/12/16 at 21:54:15, martiniss wrote:
> Couple simple tests for this? Just to get some basic coverage.

oh, right, forgot I had code here.

[martiniss, 2015-12-17 00:46:20]

docs look good. Yay spamming our users!

lgtm

[iannucci, 2015-12-17 00:53:41]

The CQ bit was checked by iannucci@chromium.org

[commit-bot: I haz the power, 2015-12-17 00:53:49]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1525923002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1525923002/40001

[commit-bot: I haz the power, 2015-12-17 00:56:49]

Description was changed from

==========
Implement Mail service.

R=dnj@chromium.org, estaab@chromium.org, martiniss@chromium.org,
vadimsh@chromium.org
BUG=https://github.com/luci/gae/issues/20
==========

to

==========
Implement Mail service.

R=dnj@chromium.org, estaab@chromium.org, martiniss@chromium.org,
vadimsh@chromium.org
BUG=https://github.com/luci/gae/issues/20

Committed:
https://github.com/luci/gae/commit/86e47afb5d2c2a20678a9003661bc9316539274b
==========

[commit-bot: I haz the power, 2015-12-17 00:56:49]

Committed patchset #3 (id:40001) as
https://github.com/luci/gae/commit/86e47afb5d2c2a20678a9003661bc9316539274b