launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Review: Approve



Diff comments:

> 
> === modified file 'lib/lp/code/templates/gitlisting.pt'
> --- lib/lp/code/templates/gitlisting.pt	2015-06-12 04:04:28 +0000
> +++ lib/lp/code/templates/gitlisting.pt	2015-12-11 06:08:34 +0000
> @@ -41,6 +41,14 @@
>            </span>
>          </div>
>        </div>
> +
> +      <div id="involvement" class="portlet"
> +           tal:define="configure_code context/menu:overview/configure_code|nothing"
> +           tal:condition="configure_code|nothing/enabled">

I think that will fail if configure_code evaluates to None.  Should be tal:condition="configure_code/enabled|nothing".

> +        <ul>
> +          <li tal:content="structure configure_code/fmt:link"></li>
> +        </ul>
> +      </div>
>      </tal:has-target>
>    </metal:side>
>    <metal:main fill-slot="main">
> 
> === modified file 'lib/lp/code/templates/product-branch-summary.pt'
> --- lib/lp/code/templates/product-branch-summary.pt	2015-06-05 03:26:02 +0000
> +++ lib/lp/code/templates/product-branch-summary.pt	2015-12-11 06:08:34 +0000
> @@ -115,18 +115,14 @@
>    <div tal:condition="context/codehosting_usage/enumvalue:UNKNOWN">
>      <div
>         tal:condition="not: context/codehosting_usage/enumvalue:LAUNCHPAD"
> -       tal:define="configure_codehosting view/configure_codehosting |
> -       nothing">
> +       tal:define="configure_code context/menu:overview/configure_code|nothing">
>        <p>
>          <a class="sprite maybe"
>             href="https://help.launchpad.net/Code";>Getting started
>            with code hosting in Launchpad</a>.</p>
>  
> -      <p tal:condition="context/required:launchpad.Edit"
> -         id="no-code-edit">
> -        <a tal:condition="configure_codehosting"
> -           tal:replace="structure configure_codehosting/fmt:link"/>
> -      </p>
> +      <p tal:condition="configure_code/enabled"

tal:condition="configure_code/enabled|nothing", since configure_code might be None.

> +         tal:content="structure configure_code/fmt:link"></p>
>      </div>
>    </div>
>  
> 
> === modified file 'lib/lp/code/templates/product-branches.pt'
> --- lib/lp/code/templates/product-branches.pt	2015-06-04 23:35:16 +0000
> +++ lib/lp/code/templates/product-branches.pt	2015-12-11 06:08:34 +0000
> @@ -39,14 +39,15 @@
>          </span>
>        </div>
>  
> -      <div id="involvement" class="portlet"
> -           tal:define="menu context/menu:branches">
> -        <p tal:define="link menu/code_import"
> -           tal:condition="link/enabled"
> -           tal:content="structure link/render"></p>
> -        <p tal:define="configure_codehosting view/configure_codehosting | nothing"
> -           tal:condition="configure_codehosting"
> -           tal:replace="structure configure_codehosting/fmt:link"></p>
> +      <div id="involvement" class="portlet">
> +        <ul>
> +          <li tal:define="link context/menu:branches/code_import"
> +              tal:condition="link/enabled"
> +              tal:content="structure link/render"></li>
> +          <li tal:define="configure_code context/menu:overview/configure_code|nothing"
> +              tal:condition="configure_code/enabled"

Ditto.

> +              tal:replace="structure configure_code/fmt:link"></li>
> +        </ul>
>        </div>
>  
>      </div>


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