Code review comment for lp:~wallyworld/launchpad/rename-private-team-795771

Revision history for this message
Ian Booth (wallyworld) wrote :

What's there is how the code was originally written, just with one check
for is_private removed. It's not possible to reject for a reason other
than a mailing list or ppa so the assert is not strictly required. The
code below will work but does change the text output if both a ppa and
mailing list exist (a bit more verbose).

I'll change it as suggested and modify any tests accordingly.

On 23/08/11 18:04, Robert Collins wrote:
> + if not has_mailing_list:
> + reason = 'has a PPA'
> + elif not has_ppa:
> + reason = 'has a mailing list'
>
> This made me blink.
>
> Perhaps:
> reasons = []
> if has_mailing_list:
> reasons.append('has a mailing list')
> if has_ppa:
> reasons.append('has a PPA')
> assert reasons, 'rejecting but no reasons found!'
> reason = ' and '.join(reasons)
>

« Back to merge proposal