launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Review: Approve



Diff comments:

> 
> === modified file 'lib/lp/code/browser/tests/test_gitrepository.py'
> --- lib/lp/code/browser/tests/test_gitrepository.py	2018-08-24 16:52:27 +0000
> +++ lib/lp/code/browser/tests/test_gitrepository.py	2018-08-29 16:40:53 +0000
> @@ -246,6 +252,47 @@
>          browser = self.getUserBrowser(url, user=user)
>          self.assertIn(project_name, browser.contents)
>  
> +    def test_view_with_active_reviews(self):
> +        repository = self.factory.makeGitRepository()
> +        git_refs = self.factory.makeGitRefs(
> +            repository,
> +            paths=["refs/heads/master", "refs/heads/1.0", "refs/tags/1.1"])
> +        self.factory.makeBranchMergeProposalForGit(
> +            target_ref=git_refs[0],
> +            set_state=BranchMergeProposalStatus.NEEDS_REVIEW)
> +        with person_logged_in(repository.owner):
> +            browser = self.getViewBrowser(repository)
> +            self.assertIsNotNone(
> +                find_tag_by_id(browser.contents, 'landing-candidates'))
> +
> +    def test_landing_candidate_count(self):
> +        source_repository = self.factory.makeGitRepository()
> +        view = create_initialized_view(source_repository, '+index')
> +
> +        self.assertEqual(view._getBranchCountText(0), 'No branches')
> +        self.assertEqual(view._getBranchCountText(1), '1 branch')
> +        self.assertEqual(view._getBranchCountText(2), '2 branches')

LP style is `self.assertEqual(expected, observed)`.

You could conceivably create branches in the test and then actually check the landing_candidate_count property, but it may not be worth the trouble.

> +
> +    def test_view_with_landing_targets(self):
> +        product = self.factory.makeProduct(name="foo", vcs=VCSType.GIT)
> +        target_repository = self.factory.makeGitRepository(target=product)
> +        source_repository = self.factory.makeGitRepository(target=product)
> +        target_git_refs = self.factory.makeGitRefs(
> +            target_repository,
> +            paths=["refs/heads/master", "refs/heads/1.0", "refs/tags/1.1"])
> +        source_git_refs = self.factory.makeGitRefs(
> +            source_repository,
> +            paths=["refs/heads/master"])
> +        self.factory.makeBranchMergeProposalForGit(
> +            target_ref=target_git_refs[0],
> +            source_ref=source_git_refs[0],
> +            set_state=BranchMergeProposalStatus.NEEDS_REVIEW)
> +        with person_logged_in(target_repository.owner):
> +            browser = self.getViewBrowser(
> +                source_repository, user=source_repository.owner)
> +            self.assertIsNotNone(
> +                find_tag_by_id(browser.contents, 'landing-targets'))
> +
>  
>  class TestGitRepositoryViewPrivateArtifacts(BrowserTestCase):
>      """Tests that Git repositories with private team artifacts can be viewed.
> 
> === added file 'lib/lp/code/templates/gitrepository-pending-merges.pt'
> --- lib/lp/code/templates/gitrepository-pending-merges.pt	1970-01-01 00:00:00 +0000
> +++ lib/lp/code/templates/gitrepository-pending-merges.pt	2018-08-29 16:40:53 +0000
> @@ -0,0 +1,35 @@
> +<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";
> +  tal:define="
> +      context_menu view/context/menu:context;
> +      features request/features"
> +  tal:condition="view/show_merge_links">
> +
> +  <h3>Branch merges</h3>

"Repository merges" perhaps?  (Not sure; they're being merged into this repository, but the things that are being merged are indeed branches.  Feel free to ignore this depending on which argument seems more compelling.)

> +  <div id="merge-links"
> +       class="actions">
> +    <div id="merge-summary">
> +
> +      <div id="landing-candidates"
> +           tal:condition="view/landing_candidates">
> +        <img src="/@@/merge-proposal-icon" />
> +        <a href="+activereviews" tal:content="structure view/landing_candidate_count_text">
> +          1 branch
> +        </a>
> +        proposed for merging into this repository.
> +
> +      </div>
> +
> +      <div id="landing-targets" tal:condition="view/landing_targets">
> +        <tal:landing-candidates repeat="mergeproposal view/landing_targets">
> +          <tal:merge-fragment
> +              tal:replace="structure mergeproposal/@@+summary-fragment"/>
> +        </tal:landing-candidates>
> +      </div>
> +
> +    </div>
> +  </div>
> +
> +</div>


-- 
https://code.launchpad.net/~twom/launchpad/git-active-review-on-more-git-repo/+merge/353884
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.