launchpad-reviewers team mailing list archive

[Ines Almeida <mp+489721@xxxxxxxxxxxxxxxxxx>]

Diff comments:

> diff --git a/lib/lp/bugs/browser/bugtask.py b/lib/lp/bugs/browser/bugtask.py
> index 081825d..91e3314 100644
> --- a/lib/lp/bugs/browser/bugtask.py
> +++ b/lib/lp/bugs/browser/bugtask.py
> @@ -1883,22 +1881,45 @@ def bugtask_sort_key(bugtask):
>              None,
>              None,
>          )
> +    # Version should only be compared to items with same object type
>      elif ISourcePackage.providedBy(bugtask.target):
>          key = (
>              bugtask.target.sourcepackagename.name,
>              bugtask.target.distribution.displayname,
> +            None,

I see. It's not the easiest to read or follow IMO.
Could you add a comment (either in front of each line or a general comment) stating what each index refers to somewhere? That will help the next person reading it understanding what each does and why some don't need it

> +            None,
> +            Version(bugtask.target.distroseries.version),
> +            None,
> +        )
> +    elif IExternalPackageSeries.providedBy(bugtask.target):
> +        key = (
> +            bugtask.target.sourcepackagename.name,
> +            bugtask.target.distribution.displayname,
> +            bugtask.target.packagetype,
> +            bugtask.target.channel,
>              Version(bugtask.target.distroseries.version),
>              None,
>              None,
>              None,
>          )
>      elif IProduct.providedBy(bugtask.target):
> -        key = (None, None, None, bugtask.target.displayname, None, None)
> +        key = (
> +            None,
> +            None,
> +            None,
> +            None,
> +            None,
> +            bugtask.target.displayname,
> +            None,
> +            None,
> +        )
>      elif IProductSeries.providedBy(bugtask.target):
>          key = (
>              None,
>              None,
>              None,
> +            None,
> +            None,
>              bugtask.target.product.displayname,
>              bugtask.target.name,
>              None,


-- 
https://code.launchpad.net/~enriqueesanchz/launchpad/+git/launchpad/+merge/489721
Your team Launchpad code reviewers is requested to review the proposed merge of ~enriqueesanchz/launchpad:add-external-package-series into launchpad:master.