Code review comment for lp:~jelmer/launchpad/bug471148-ui

Revision history for this message
Abel Deuring (adeuring) wrote :

(14:46:44) adeuring: jelmer: could you add doc strings to _get_arm_builds_enabled() and _get_arm_builds_disabled()?
(14:47:36) adeuring: jelmer: sorry... i mean _set_arm_builds_enabled() instead of the "disabled"
(14:47:54) jelmer: adeuring: ok
(14:48:34) abentley [~<email address hidden>] hat den Raum betreten.
(14:49:15) adeuring: jelmer: there is an "elif" in set_arms_builds_enabled() without a final "else". Could you add such an "else: pass", together with a comment why nothing needs to be done in this case?
(14:51:04) adeuring: jelmer: and (sorry about all these nitpick for one method...): The comment "a link is required but it is present" is a bit enigmatic, at least for me. What about sonething like "Arms builds are already enabled"?
(14:51:41) jelmer: adeuring: Sure, that's more sensible indeed.

review: Approve (code)

« Back to merge proposal