Code review comment for lp:~rogpeppe/tomb/update-api

Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

https://codereview.appspot.com/5755055/diff/3001/tomb.go
File tomb.go (right):

https://codereview.appspot.com/5755055/diff/3001/tomb.go#newcode44
tomb.go:44: // created or already alive. Once Kill is called with an
s/Kill/Kill or Killf/, as before please.

https://codereview.appspot.com/5755055/diff/3001/tomb.go#newcode80
tomb.go:80: if t.state == uninitialized {
if t.dead == nil {
     t.dead = make(...)
     t.dying = make(...)
     t.reason = ErrStillRunning
}

We don't need "state".

https://codereview.appspot.com/5755055/diff/3001/tomb.go#newcode130
tomb.go:130: reason = errCleanKill
nil itself is errCleanKill. We don't need a second way to flag this.

https://codereview.appspot.com/5755055/diff/3001/tomb.go#newcode136
tomb.go:136: if t.state == running {
if t.reason != ErrStillRunning

https://codereview.appspot.com/5755055/diff/3001/tomb.go#newcode162
tomb.go:162: }
The switch is unnecessary with the suggested change. Just return
t.reason at all times.

https://codereview.appspot.com/5755055/

« Back to merge proposal