Code review comment for lp:~mabac/svammel/bug-749561-set-series-and-milestone

Revision history for this message
Mattias Backman (mabac) wrote :

> Sorry for the delay in reviewing this.

No problem. I had left it in status "work in progress" until recently too.
>
> 8       +class AttributeError(Exception):
>
> Could we avoid re-defining a standard exception, that's only going to lead to confusion.

That's my mistake, didn't check for a standard exception with that
name. Removed.

> 59      +                set_bugtask_milestone(bug.bug_tasks[0:2][-1], milestone)
>
> Like Mattias I am uneasy about this, it looks very fragile. Is there
> some rule you can verbalise about which task is needed at this point?
> If so then we can try and code that up, rather than relying on ordering
> of the tasks here.

That rule should be to use the only bug_task if no series has been
nominated, and to use the bug_task that actually is targeted to the
specified series otherwise. Trivial now that I think about it, thanks.
:) I've changed this as well.

Thanks,

Mattias

« Back to merge proposal