launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Review: Approve

Ah, thank you for sorting out the breadcrumbs - I'd been wondering what the problem was there but hadn't quite managed to figure it out.  Just one nit that could hardly be any smaller.

Diff comments:

> === modified file 'lib/lp/code/browser/configure.zcml'
> --- lib/lp/code/browser/configure.zcml	2015-06-12 08:47:42 +0000
> +++ lib/lp/code/browser/configure.zcml	2015-06-12 08:47:43 +0000
> @@ -765,9 +765,6 @@
>              name="+portlet-privacy"
>              template="../templates/gitrepository-portlet-privacy.pt"/>
>          <browser:page
> -            name="++repository-information"
> -            template="../templates/gitrepository-information.pt"/>
> -        <browser:page
>              name="++repository-management"
>              template="../templates/gitrepository-management.pt"/>
>      </browser:pages>
> 
> === modified file 'lib/lp/code/browser/gitrepository.py'
> --- lib/lp/code/browser/gitrepository.py	2015-06-10 16:41:28 +0000
> +++ lib/lp/code/browser/gitrepository.py	2015-06-12 08:47:43 +0000
> @@ -93,7 +93,7 @@
>  
>      @property
>      def inside(self):
> -        return self.context.unique_name.split("/")[-1]
> +        return self.context.target
>  
>  
>  class GitRepositoryNavigation(Navigation):
> 
> === modified file 'lib/lp/code/browser/tests/test_gitlisting.py'
> --- lib/lp/code/browser/tests/test_gitlisting.py	2015-06-12 08:47:42 +0000
> +++ lib/lp/code/browser/tests/test_gitlisting.py	2015-06-12 08:47:43 +0000
> @@ -61,8 +61,8 @@
>  
>          # Clone instructions for the default repo are present.
>          self.assertEqual(
> -            'git://git.launchpad.dev/%s' % self.target_path,
> -            soup.find(attrs={'class': 'anon-url'}).find(text=True))
> +            'https://git.launchpad.dev/%s' % self.target_path,
> +            soup.find(attrs={'class': 'https-url'}).find(text=True))
>          self.assertEqual(
>              'https://git.launchpad.dev/~foowner/%s/+git/foo'
>              % self.target_path,
> @@ -209,8 +209,8 @@
>  
>          # Clone instructions for the default repo are present.
>          self.assertEqual(
> -            'git://git.launchpad.dev/~dev/%s' % self.target_path,
> -            soup.find(attrs={'class': 'anon-url'}).find(text=True))
> +            'https://git.launchpad.dev/~dev/%s' % self.target_path,
> +            soup.find(attrs={'class': 'https-url'}).find(text=True))
>          self.assertEqual(
>              'https://git.launchpad.dev/~dev/%s/+git/foo' % self.target_path,
>              soup.find(text='Browse the code').parent['href'])
> 
> === modified file 'lib/lp/code/interfaces/gitrepository.py'
> --- lib/lp/code/interfaces/gitrepository.py	2015-06-06 08:49:54 +0000
> +++ lib/lp/code/interfaces/gitrepository.py	2015-06-12 08:47:43 +0000
> @@ -213,11 +213,11 @@
>          "The identity of this repository: a VCS-independent synonym for "
>          "git_identity.")
>  
> -    anon_url = Attribute(
> -        "An anonymous (git://) URL for this repository, or None in the case "
> -        "of private repositories.")
> +    git_https_url = Attribute(
> +        "An HTTPS URL for this repository, or None in the case of private "
> +        "repositories.")
>  
> -    ssh_url = Attribute("A git+ssh:// URL for this repository.")
> +    git_ssh_url = Attribute("A git+ssh:// URL for this repository.")
>  
>      refs = exported(CollectionField(
>          title=_("The references present in this repository."),
> 
> === modified file 'lib/lp/code/model/gitrepository.py'
> --- lib/lp/code/model/gitrepository.py	2015-06-06 08:49:54 +0000
> +++ lib/lp/code/model/gitrepository.py	2015-06-12 08:47:43 +0000
> @@ -342,16 +342,18 @@
>              config.codehosting.git_browse_root, self.unique_name)
>  
>      @property
> -    def anon_url(self):
> +    def git_https_url(self):
>          """See `IGitRepository`."""
> +        # XXX wgrant 2015-06-12: This guard should be removed once we
> +        # support Git HTTPS auth.
>          if self.visibleByUser(None):
>              return urlutils.join(
> -                config.codehosting.git_anon_root, self.shortened_path)
> +                config.codehosting.git_browse_root, self.shortened_path)
>          else:
>              return None
>  
>      @property
> -    def ssh_url(self):
> +    def git_ssh_url(self):
>          """See `IGitRepository`."""
>          return urlutils.join(
>              config.codehosting.git_ssh_root, self.shortened_path)
> 
> === modified file 'lib/lp/code/model/tests/test_gitrepository.py'
> --- lib/lp/code/model/tests/test_gitrepository.py	2015-06-06 08:49:54 +0000
> +++ lib/lp/code/model/tests/test_gitrepository.py	2015-06-12 08:47:43 +0000
> @@ -734,12 +734,12 @@
>              config.codehosting.git_browse_root, repository.unique_name)
>          self.assertEqual(expected_url, repository.getCodebrowseUrl())
>  
> -    def test_anon_url_for_public(self):
> +    def test_git_https_url_for_public(self):
>          # Public repositories have an anonymous URL, visible to anyone.
>          repository = self.factory.makeGitRepository()
>          expected_url = urlutils.join(
> -            config.codehosting.git_anon_root, repository.shortened_path)
> -        self.assertEqual(expected_url, repository.anon_url)
> +            config.codehosting.git_browse_root, repository.shortened_path)
> +        self.assertEqual(expected_url, repository.git_https_url)
>  
>      def test_anon_url_not_for_private(self):
>          # Private repositories do not have an anonymous URL.
> @@ -747,14 +747,14 @@
>          repository = self.factory.makeGitRepository(
>              owner=owner, information_type=InformationType.USERDATA)
>          with person_logged_in(owner):
> -            self.assertIsNone(repository.anon_url)
> +            self.assertIsNone(repository.git_https_url)
>  
> -    def test_ssh_url_for_public(self):
> +    def test_git_ssh_url_for_public(self):
>          # Public repositories have an SSH URL.
>          repository = self.factory.makeGitRepository()
>          expected_url = urlutils.join(
>              config.codehosting.git_ssh_root, repository.shortened_path)
> -        self.assertEqual(expected_url, repository.ssh_url)
> +        self.assertEqual(expected_url, repository.git_ssh_url)
>  
>      def test_ssh_url_for_private(self):
>          # Private repositories have an SSH URL.
> @@ -764,7 +764,7 @@
>          with person_logged_in(owner):
>              expected_url = urlutils.join(
>                  config.codehosting.git_ssh_root, repository.shortened_path)
> -            self.assertEqual(expected_url, repository.ssh_url)
> +            self.assertEqual(expected_url, repository.git_ssh_url)
>  
>  
>  class TestGitRepositoryNamespace(TestCaseWithFactory):
> 
> === modified file 'lib/lp/code/templates/gitrepository-index.pt'
> --- lib/lp/code/templates/gitrepository-index.pt	2015-06-04 16:57:58 +0000
> +++ lib/lp/code/templates/gitrepository-index.pt	2015-06-12 08:47:43 +0000
> @@ -24,12 +24,7 @@
>  </metal:side>
>  
>  <tal:registering metal:fill-slot="registering">
> -  Created by
> -    <tal:registrant replace="structure context/registrant/fmt:link" />
> -  on
> -    <tal:created-on replace="structure context/date_created/fmt:date" />
> -  and last modified on
> -    <tal:last-modified replace="structure context/date_last_modified/fmt:date" />
> +  Owned by <tal:owner replace="structure context/owner/fmt:link" />.

The trailing full stop here is anomalous, I think.

>  </tal:registering>
>  
>  <div metal:fill-slot="main">
> @@ -46,14 +41,6 @@
>    </div>
>  
>    <div class="yui-g">
> -    <div id="repository-info" class="portlet">
> -      <h2>Repository information</h2>
> -      <tal:repository-info
> -          replace="structure context/@@++repository-information" />
> -    </div>
> -  </div>
> -
> -  <div class="yui-g">
>      <div id="repository-branches" class="portlet"
>           tal:define="branches view/branches">
>        <h2>Branches</h2>
> 
> === removed file 'lib/lp/code/templates/gitrepository-information.pt'
> --- lib/lp/code/templates/gitrepository-information.pt	2015-03-04 16:49:42 +0000
> +++ lib/lp/code/templates/gitrepository-information.pt	1970-01-01 00:00:00 +0000
> @@ -1,18 +0,0 @@
> -<div
> -  xmlns:tal="http://xml.zope.org/namespaces/tal";
> -  xmlns:metal="http://xml.zope.org/namespaces/metal";
> -  xmlns:i18n="http://xml.zope.org/namespaces/i18n";>
> -
> -  <div class="two-column-list">
> -    <dl id="owner">
> -      <dt>Owner:</dt>
> -      <dd tal:content="structure context/owner/fmt:link" />
> -    </dl>
> -
> -    <dl id="partof" tal:condition="context/target">
> -      <dt>Target:</dt>
> -      <dd tal:content="structure context/target/fmt:link" />
> -    </dl>
> -  </div>
> -
> -</div>
> 
> === modified file 'lib/lp/code/templates/gitrepository-management.pt'
> --- lib/lp/code/templates/gitrepository-management.pt	2015-05-01 13:18:54 +0000
> +++ lib/lp/code/templates/gitrepository-management.pt	2015-06-12 08:47:43 +0000
> @@ -7,15 +7,15 @@
>    <dl id="clone-url">
>      <dt>Get this repository:</dt>
>      <dd>
> -      <tal:anonymous condition="context/anon_url">
> +      <tal:anonymous condition="context/git_https_url">
>          <tt class="command">
> -          git clone <span class="anon-url" tal:content="context/anon_url" />
> +          git clone <span class="https-url" tal:content="context/git_https_url" />
>          </tt>
>          <br />
>        </tal:anonymous>
>        <tal:ssh condition="view/user">
>          <tt class="command">
> -          git clone <span class="ssh-url" tal:content="context/ssh_url" />
> +          git clone <span class="ssh-url" tal:content="context/git_ssh_url" />
>          </tt>
>        </tal:ssh>
>      </dd>
> 


-- 
https://code.launchpad.net/~wgrant/launchpad/git-view-cleanup/+merge/261818
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.