launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Review: Approve



Diff comments:

> === modified file 'lib/lp/registry/browser/product.py'
> === modified file 'lib/lp/registry/browser/productseries.py'
> --- lib/lp/registry/browser/product.py	2015-07-02 07:18:35 +0000
> +++ lib/lp/registry/browser/product.py	2015-07-02 09:20:35 +0000
> @@ -1040,11 +1040,13 @@
>                  return None
>          elif (self.context.vcs == VCSType.BZR and
>          self.context.development_focus.branch):
> -            return ("{base_url}/{name} bzr "
> -                    "{browse_root}{name}").format(
> -                        base_url=config.vhost.mainsite.hostname,
> -                        name=self.context.development_focus.branch.unique_name,
> -                        browse_root=config.codehosting.supermirror_root)
> +            return (

I'd make this be {base_url}/{product} instead; this would match the git case, and once you have more than one thing that has a name then "name" can be a bit of a confusing term to use.

> +                "{base_url}/{name} bzr "
> +                "{browse_root}{branch}").format(
> +                    base_url=config.vhost.mainsite.hostname,
> +                    name=self.context.name,
> +                    branch=self.context.development_focus.branch.unique_name,
> +                    browse_root=config.codehosting.supermirror_root)
>          else:
>              return None
>  
> === modified file 'lib/lp/registry/browser/tests/test_product.py'
> --- lib/lp/registry/browser/productseries.py	2015-07-02 03:01:04 +0000
> +++ lib/lp/registry/browser/productseries.py	2015-07-02 09:20:35 +0000
> @@ -387,9 +387,10 @@
>          """
>          if (self.context.product.vcs == VCSType.BZR and
>              self.context.product.development_focus.branch):
> -            return ("{base_url}/{name} bzr {root}{name}").format(
> +            return ("{base_url}/{name} bzr {root}{branch}").format(

Same here, {base_url}/{product} instead.

>                  base_url=config.vhost.mainsite.hostname,
> -                name=self.context.branch.unique_name,
> +                name=self.context.product.name,
> +                branch=self.context.branch.unique_name,
>                  root=config.codehosting.supermirror_root)
>          else:
>              return None


-- 
https://code.launchpad.net/~blr/launchpad/golang-meta-import-fix/+merge/263625
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.