launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

Review: Needs Fixing code



Diff comments:

> === modified file 'lib/lp/registry/browser/product.py'
> --- lib/lp/registry/browser/product.py	2015-06-22 03:15:41 +0000
> +++ lib/lp/registry/browser/product.py	2015-06-22 03:15:42 +0000
> @@ -165,7 +165,7 @@
>  from lp.code.interfaces.gitcollection import IGitCollection
>  from lp.code.interfaces.gitrepository import IGitRepositorySet
>  from lp.code.browser.vcslisting import TargetDefaultVCSNavigationMixin
> -
> +from lp.code.interfaces.gitrepository import IGitRepositorySet
>  from lp.registry.browser import (
>      add_subscribe_link,
>      BaseRdfView,
> @@ -221,6 +221,7 @@
>      stepto,
>      structured,
>      )
> +from lp.services.config import config
>  from lp.services.webapp.authorization import check_permission
>  from lp.services.webapp.batching import BatchNavigator
>  from lp.services.webapp.breadcrumb import Breadcrumb
> @@ -1026,6 +1027,29 @@
>      def requestCountry(self):
>          return ICountry(self.request, None)
>  
> +    @property
> +    def golang_import_spec(self):
> +        """Meta string for golang remote import path.
> +        See: https://golang.org/cmd/go/#hdr-Remote_import_paths
> +        """
> +        if self.context.vcs == VCSType.GIT:
> +            repo = getUtility(IGitRepositorySet).getDefaultRepository(
> +                self.context)
> +            if repo:
> +                return "{base_url}/{product} git {git_https_url}".format(
> +                    base_url=config.vhost.mainsite.hostname,
> +                    product=self.context.name,
> +                    git_https_url=repo.git_https_url)
> +            else:
> +                return None
> +        elif self.context.vcs == VCSType.BZR:
> +            return "{base_url}/{product} bzr {codebrowse_root}{product}".format(
> +                base_url=config.vhost.mainsite.hostname,
> +                product=self.context.name,
> +                codebrowse_root=config.codehosting.secure_codebrowse_root)

secure_codebrowse_root (https://bazaar.launchpad.net) does not serve branches in a format that Bazaar can read. Only the non-HTTPS service does, so you probably want config.codehosting.supermirror_root. It also doesn't accept the product/series aliases, so you need to include the branch's full unique_name.

See BranchRefNavigation.traverse_branch's "location" case for how this works now.

> +        else:
> +            return None
> +
>      def browserLanguages(self):
>          return browser_languages(self.request)
>  
> 
> === modified file 'lib/lp/registry/browser/productseries.py'
> --- lib/lp/registry/browser/productseries.py	2015-06-22 03:15:41 +0000
> +++ lib/lp/registry/browser/productseries.py	2015-06-22 03:15:42 +0000
> @@ -80,11 +80,13 @@
>      RegistryDeleteViewMixin,
>      StatusCount,
>      )
> +from lp.code.interfaces.gitrepository import IGitRepositorySet
>  from lp.registry.browser.pillar import (
>      InvolvedMenu,
>      PillarInvolvementView,
>      )
>  from lp.registry.browser.product import ProductSetBranchView
> +from lp.registry.enums import VCSType
>  from lp.registry.errors import CannotPackageProprietaryProduct
>  from lp.registry.interfaces.packaging import (
>      IPackaging,
> @@ -92,6 +94,7 @@
>      )
>  from lp.registry.interfaces.productseries import IProductSeries
>  from lp.registry.interfaces.series import SeriesStatus
> +from lp.services.config import config
>  from lp.services.propertycache import cachedproperty
>  from lp.services.webapp import (
>      ApplicationMenu,
> @@ -379,6 +382,21 @@
>          """The country associated with the IP of the request."""
>          return ICountry(self.request, None)
>  
> +    @property
> +    def golang_import_spec(self):
> +        """Meta string for golang remote import path.
> +        See: https://golang.org/cmd/go/#hdr-Remote_import_paths
> +        """
> +        if self.context.product.vcs == VCSType.BZR:
> +            return ("{base_url}/{product}/{series} bzr "
> +                    "{root}{product}/{series}").format(
> +                        base_url=config.launchpad.non_restricted_hostname,
> +                        product=self.context.product.name,
> +                        root=config.codehosting.secure_codebrowse_root,
> +                        series=self.context.name)

Same concerns as the Product case.

> +        else:
> +            return None
> +
>      def browserLanguages(self):
>          """The languages the user's browser requested."""
>          return browser_languages(self.request)
> 
> === modified file 'lib/lp/registry/browser/tests/test_product.py'
> --- lib/lp/registry/browser/tests/test_product.py	2015-06-22 03:15:41 +0000
> +++ lib/lp/registry/browser/tests/test_product.py	2015-06-22 03:15:42 +0000
> @@ -27,6 +27,7 @@
>      PROPRIETARY_INFORMATION_TYPES,
>      ServiceUsage,
>      )
> +from lp.code.interfaces.gitrepository import IGitRepositorySet
>  from lp.registry.browser.product import (
>      ProjectAddStepOne,
>      ProjectAddStepTwo,
> @@ -295,6 +296,40 @@
>          super(TestProductView, self).setUp()
>          self.product = self.factory.makeProduct(name='fnord')
>  
> +    def test_golang_meta_renders_git(self):
> +        # ensure golang meta import path is rendered if project has
> +        # git default vcs.
> +        # See: https://golang.org/cmd/go/#hdr-Remote_import_paths
> +        repo = self.factory.makeGitRepository()
> +        view = create_initialized_view(repo.target, '+index')
> +        with person_logged_in(repo.target.owner):
> +            getUtility(IGitRepositorySet).setDefaultRepository(
> +                target=repo.target, repository=repo)
> +            repo.target.vcs = VCSType.GIT
> +
> +        golang_import = '{base}/{product_name} git {repo_url}'.format(
> +            base=config.vhost.mainsite.hostname,
> +            product_name=repo.target.name,
> +            repo_url=repo.git_https_url
> +            )
> +        self.assertEqual(golang_import, view.golang_import_spec)
> +
> +    def test_golang_meta_renders_bzr(self):
> +        # ensure golang meta import path is rendered if project has
> +        # bzr default vcs.
> +        # See: https://golang.org/cmd/go/#hdr-Remote_import_paths
> +        branch = self.factory.makeBranch()
> +        view = create_initialized_view(branch.product, '+index')
> +        with person_logged_in(branch.product.owner):
> +            branch.product.vcs = VCSType.BZR
> +
> +        golang_import = '{base}/{name} bzr {repo_url}{name}'.format(
> +            base=config.vhost.mainsite.hostname,
> +            name=branch.target.name,
> +            repo_url=config.codehosting.secure_codebrowse_root
> +            )
> +        self.assertEqual(golang_import, view.golang_import_spec)
> +
>      def test_show_programming_languages_without_languages(self):
>          # show_programming_languages is false when there are no programming
>          # languages set.
> 
> === modified file 'lib/lp/registry/browser/tests/test_productseries_views.py'
> --- lib/lp/registry/browser/tests/test_productseries_views.py	2013-04-03 03:09:04 +0000
> +++ lib/lp/registry/browser/tests/test_productseries_views.py	2015-06-22 03:15:42 +0000
> @@ -8,6 +8,7 @@
>  
>  import soupmatchers
>  from testtools.matchers import Not
> +from zope.component import getUtility
>  from zope.security.proxy import removeSecurityProxy
>  
>  from lp.app.enums import InformationType
> @@ -15,6 +16,9 @@
>      BugTaskStatus,
>      BugTaskStatusSearch,
>      )
> +from lp.code.interfaces.gitrepository import IGitRepositorySet
> +from lp.registry.enums import VCSType
> +from lp.services.config import config
>  from lp.services.webapp import canonical_url
>  from lp.testing import (
>      BrowserTestCase,
> @@ -30,6 +34,24 @@
>  
>      layer = DatabaseFunctionalLayer
>  
> +    def test_golang_meta_renders(self):
> +        # ensure golang meta import path is rendered if project has
> +        # bzr default vcs.
> +        # See: https://golang.org/cmd/go/#hdr-Remote_import_paths
> +        branch = self.factory.makeBranch()
> +        view = create_initialized_view(branch.product.development_focus,
> +                                       '+index')
> +        with person_logged_in(branch.product.owner):
> +            branch.product.vcs = VCSType.BZR
> +
> +        golang_import = ("{base}/{name}/{series} bzr "
> +                         "{repo_url}{name}/{series}").format(
> +                             base=config.vhost.mainsite.hostname,
> +                             name=branch.product.name,
> +                             repo_url=config.codehosting.secure_codebrowse_root,
> +                             series=branch.product.development_focus.name)
> +        self.assertEqual(golang_import, view.golang_import_spec)
> +
>      def test_information_type_public(self):
>          # A ProductSeries view should include its information_type,
>          # which defaults to Public for new projects.
> 
> === modified file 'lib/lp/registry/templates/product-index.pt'
> --- lib/lp/registry/templates/product-index.pt	2015-06-22 03:15:41 +0000
> +++ lib/lp/registry/templates/product-index.pt	2015-06-22 03:15:42 +0000
> @@ -25,6 +25,9 @@
>            });
>        </script>
>      </tal:uses_launchpad_bugtracker>
> +
> +    <meta name="go-import" tal:condition="view/golang_import_spec"
> +          tal:attributes="content view/golang_import_spec" />
>    </tal:head-epilogue>
>  </head>
>  
> 
> === modified file 'lib/lp/registry/templates/productseries-index.pt'
> --- lib/lp/registry/templates/productseries-index.pt	2012-10-16 15:12:09 +0000
> +++ lib/lp/registry/templates/productseries-index.pt	2015-06-22 03:15:42 +0000
> @@ -25,6 +25,8 @@
>            });
>        </script>
>      </tal:uses_launchpad_bugtracker>
> +    <meta name="go-import" tal:condition="view/golang_import_spec"
> +          tal:attributes="content view/golang_import_spec" />
>    </metal:block>
>  
>      <tal:heading metal:fill-slot="heading">
> 


-- 
https://code.launchpad.net/~blr/launchpad/project-meta-go-import/+merge/262550
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.