launchpad-reviewers team mailing list archive

[Ioana Lasc <mp+415250@xxxxxxxxxxxxxxxxxx>]

Guruprasad thanks for the review and good comments! I've replied inline, we do have an additional (approach issue) that Colin noticed yesterday: I went ad litteram by the spec and it seems that's really the desired behavior we want to implement for this, we'll get more details when Colin has a chance to review.

Diff comments:

> diff --git a/lib/lp/bugs/interfaces/bugtask.py b/lib/lp/bugs/interfaces/bugtask.py
> index 5d22a40..027612d 100644
> --- a/lib/lp/bugs/interfaces/bugtask.py
> +++ b/lib/lp/bugs/interfaces/bugtask.py
> @@ -224,6 +224,12 @@ class BugTaskStatus(DBEnumeratedType):
>          The fix was released.
>          """)
>  
> +    DOES_NOT_EXIST = DBItem(35, """
> +        Does not exist
> +
> +        The package does not exist in this series.

Good point, although I'm not sure what the best phrasing for this will be, I definitely agree that the current one isn't. I'll propose something else with the next update.

> +        """)
> +
>      UNKNOWN = DBItem(999, """
>          Unknown
>  
> diff --git a/lib/lp/bugs/model/bugtask.py b/lib/lp/bugs/model/bugtask.py
> index 7b31d6d..b468c5e 100644
> --- a/lib/lp/bugs/model/bugtask.py
> +++ b/lib/lp/bugs/model/bugtask.py
> @@ -358,8 +358,8 @@ def validate_target(bug, target, retarget_existing=True,
>              try:
>                  target.distribution.guessPublishedSourcePackageName(
>                      target.sourcepackagename.name)
> -            except NotFoundError as e:
> -                raise IllegalTarget(e.args[0])
> +            except NotFoundError:
> +                return BugTaskStatus.DOES_NOT_EXIST

If we return the exception that will propagate and we end up not creating the task and we now want to record the DNE against a task and show that we've considered a possible bug target and determined that the code doesn't exist in that series. I might have miss-understood the spec but I believe that's how we want to go about this.

>  
>      legal_types = target.pillar.getAllowedBugInformationTypes()
>      new_pillar = target.pillar not in bug.affected_pillars


-- 
https://code.launchpad.net/~ilasc/launchpad/+git/launchpad/+merge/415250
Your team Launchpad code reviewers is requested to review the proposed merge of ~ilasc/launchpad:add-dne-bug-task-status into launchpad:master.