Code review comment for lp:~jtv/launchpad/bug-613821

<danilos> jtv, did addOrUpdateEntry signature change? (or is it just whitespaces?)
<jtv> danilos: whitespace.
<danilos> jtv, cool
<jtv> I promise.
* danilos rechecks
<danilos> jtv, does _getMatchingEntry return only an already approved entry? (i.e. it will not return "itself")
<jtv> danilos: it's only called when looking for the more specific match that the approver's pofile/potemplate guess will produce.
<jtv> So it won't produce the entry that you're already looking at: you'd be looking for an entry with the same pofile/potemplate, and in that case the caller shortcuts.
<danilos> jtv, it'd be nice to return an explicit False in _attemptToApprove when entry.status != NEEDS_REVIEW
<jtv> danilos: whoops, sorry :)
<danilos> jtv, also, your comment about the 'checking at the very end if it was already approved' confused me a bit :)
<danilos> jtv, old code was not even considering that problem, it was checking if approval succeeded instead
<danilos> jtv, basically what you do with a check on the returned value from _attemptToApprove
<danilos> jtv, anyway, looks good other than that minor point above, r=danilo
<jtv> danilos: thanks. I meant the part where it checked at the end whether the entry's status was APPROVED. Not too sane. :-)
 danilos: could you land it for me?
<danilos> jtv, right, but that's your "if success" check, roughly the same thing