launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Review: Approve



Diff comments:

> diff --git a/lib/lp/registry/javascript/sharing/sharingdetails.js b/lib/lp/registry/javascript/sharing/sharingdetails.js
> index 0ac4f32..22db012 100644
> --- a/lib/lp/registry/javascript/sharing/sharingdetails.js
> +++ b/lib/lp/registry/javascript/sharing/sharingdetails.js
> @@ -232,7 +304,7 @@ ns.SharingDetailsTable = Y.Base.create('sharingDetailsTable', Y.Widget, [], {
>                      .appendChild('<td colspan="3"></td>')
>                      .setContent(
>                          "There are no shared bugs, Bazaar branches, " +
> -                        "Git repositories, or blueprints.");
> +                        "Git repositories, Snaps, OCI recipes or blueprints.");

This text should be synced with that in pillar-sharing-details.pt.

>              }
>          };
>          var anim_duration = this.get('anim_duration');
> diff --git a/lib/lp/registry/templates/pillar-sharing-details.pt b/lib/lp/registry/templates/pillar-sharing-details.pt
> index dc907dd..8a9a2fe 100644
> --- a/lib/lp/registry/templates/pillar-sharing-details.pt
> +++ b/lib/lp/registry/templates/pillar-sharing-details.pt
> @@ -26,21 +26,32 @@
>    <div metal:fill-slot="main">
>  
>      <div id="observer-summary">
> -      <p>
> -      <tal:bugs replace="view/shared_bugs_count">0</tal:bugs> bugs,
> -      <tal:branches replace="view/shared_branches_count">0</tal:branches> Bazaar branches,
> -      <tal:gitrepositories replace="view/shared_gitrepositories_count">0</tal:gitrepositories> Git repositories,
> -      <tal:snaps replace="view/shared_snaps_count">0</tal:snaps> snaps,
> -      and  <tal:specifications
> -      replace="view/shared_specifications_count">0</tal:specifications>
> -      blueprints shared with <tal:name replace="view/person/displayname">
> -      grantee</tal:name>.<br />
> -
>        <tal:is-team condition="view/person/is_team">
>          <tal:members>3</tal:members> team members can view these bugs,
> -        Bazaar branches, Git repositories, and blueprints.
> +        Bazaar branches, Git repositories, snaps recipes, OCI recipes and

"snap recipes", not "snaps recipes".

On second reading, I also wonder whether this summary text should be replaced with "... team members can view these artifacts", since spelling it all out is getting rather unwieldy.

Finally, I wonder how apparently nobody has previously noticed that "3" is hardcoded here, and is presumably untested.  Feel free to file a bug about that if it isn't trivial to fix in this branch, though.

> +        blueprints.
>        </tal:is-team>
> -      </p>
> +      Shared with <tal:name replace="view/person/displayname">grantee</tal:name>:
> +      <ul class="listing">

In your screenshot, this doesn't seem to be styled as a list (with bullet points or similar).  Can you find out why that is?

> +        <li tal:condition="view/shared_bugs_count">
> +          <span tal:replace="view/shared_bugs_count" /> bugs
> +        </li>
> +        <li tal:condition="view/shared_branches_count">
> +          <span tal:replace="view/shared_branches_count" /> Bazaar branches
> +        </li>
> +        <li tal:condition="view/shared_gitrepositories_count">
> +          <span tal:replace="view/shared_gitrepositories_count" /> Git repositories
> +        </li>
> +        <li tal:condition="view/shared_ocirecipe_count">
> +          <span tal:replace="view/shared_ocirecipe_count" /> OCI recipes
> +        </li>
> +        <li tal:condition="view/shared_snaps_count">
> +          <span tal:replace="view/shared_snaps_count" /> snap recipes
> +        </li>
> +        <li tal:condition="view/shared_specifications_count">
> +          <span tal:replace="view/shared_specifications_count" /> blueprints
> +        </li>
> +      </ul>
>      </div>
>  
>      <table id="shared-table" class="listing sortable">


-- 
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/400059
Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:ocirecipe-edit-info-type-ui.