← Back to team overview

launchpad-reviewers team mailing list archive

Re: [Merge] lp:~rharding/launchpad/bugnom_874250 into lp:launchpad

 

Review: Approve

This looks good to me. I have some nitpicks below, but nothing blocking. 

> === modified file 'lib/lp/bugs/model/bug.py'
> --- lib/lp/bugs/model/bug.py	2012-05-09 06:12:52 +0000
> +++ lib/lp/bugs/model/bug.py	2012-05-10 14:54:18 +0000
> @@ -1237,6 +1237,11 @@
>          """See `IBug`."""
>          return getUtility(IBugTaskSet).createTask(self, owner, target)
>  
> +    def addManyTasks(self, owner, targets):
> +        """See `IBug`."""
> +        new_tasks = getUtility(IBugTaskSet).createManyTasks(self, owner, targets)
> +        return new_tasks
> +

Is there any reason to not just have this be?

    def addManyTasks(self, owner, targets):
        """See `IBug`."""
        return getUtility(IBugTaskSet).createManyTasks(self, owner, targets)
 
> === modified file 'lib/lp/bugs/model/bugtask.py'
> --- lib/lp/bugs/model/bugtask.py	2012-05-09 13:39:12 +0000
> +++ lib/lp/bugs/model/bugtask.py	2012-05-10 14:54:18 +0000
> @@ -1604,11 +1603,8 @@
>          params = BugTaskSearchParams(user, **kwargs)
>          return self.search(params)
>  
> -    def createTask(self, bug, owner, target,
> -                   status=IBugTask['status'].default,
> -                   importance=IBugTask['importance'].default,
> -                   assignee=None, milestone=None):
> -        """See `IBugTaskSet`."""
> +    def _init_new_task(self, bug, owner, target, status, importance, assignee,
> +                       milestone):
>          if not status:
>              status = IBugTask['status'].default
>          if not importance:
 
This is a bit nitpicky, but methods, private or otherwise, should still
be camelCase.


-- 
https://code.launchpad.net/~rharding/launchpad/bugnom_874250/+merge/105317
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.


References