j.c.sackett has proposed merging
lp:~jcsackett/launchpad/branch-collection-links-652126 into lp:launchpad/devel.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
#652126 branch collection action portlet is missing links
https://bugs.launchpad.net/bugs/652126
Summary
=======
This branch deals with bad expectations on product branches. There's a set of
statistical information in a side portlet that has some links, but not all
links. Given the side portlets tend to be for actions, it seems odd that the
remaining statistics aren't actionable.
To deal with this, the branch moves the stats into text on the main page,
keeping the useful information, but avoiding breaking the expectations.
There is still one link to active-reviews in the new inline text; I'm open to
the suggestion that active reviews still belong in a side portlet.
Proposed fix
============
Move the information in the sidebar into the main page, and eliminate the
problematic part of the sidebar.
Preimplementation talk
======================
Spoke with Paul Hummer. Discussed intent of the statistics portlet and
concluded it could be moved out of the sidebar portlet and into the main page.
Additionally discussed not linking to active reviews if there are no active
reviews.
Implementation details
======================
As in proposed. In the process of moving the portlet out of the side bar and
into the main page, the two portlets showing the statistics were merged into
one, since the existance of two portlets used *only* to present that data
seemed unnecessary at best.
Tests
=====
bin/test -vvcm lp.code.browser.tests.test_product
bin/test -t product-branches
Right now product-branches isn't passing b/c the want and received text are
disagreeing on line breaks; I will fix this before submitting, but wanted to
get this moving forward on review. If you run product-branches, you will see
the text agrees, but the line breaks are failing. I'm investigating clean ways
to improve the test.
Screenshots
===========
The original appearance is here:
http://people.canonical.com/~jc/images/oldstats.png
The new appearance is here: http://people.canonical.com/~jc/images/newstats.png
Lint
====
make lint output:
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/code/stories/branches/xx-product-branches.txt
lib/lp/code/templates/product-branches.pt
./lib/lp/code/stories/branches/xx-product-branches.txt
41: want exceeds 78 characters.
There doesn't seem to be a clean way to shorten the want in this test.
--
https://code.launchpad.net/~jcsackett/launchpad/branch-collection-links-652126/+merge/38456
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~jcsackett/launchpad/branch-collection-links-652126 into lp:launchpad/devel.
=== modified file 'lib/lp/code/browser/configure.zcml'
--- lib/lp/code/browser/configure.zcml 2010-10-06 18:53:53 +0000
+++ lib/lp/code/browser/configure.zcml 2010-10-14 20:29:43 +0000
@@ -974,12 +974,6 @@
permission="zope.Public"
name="+portlet-product-codestatistics"
template="../templates/product-portlet-codestatistics.pt"/>
- <browser:page
- for="lp.registry.interfaces.product.IProduct"
- class="lp.code.browser.branchlisting.ProductBranchStatisticsView"
- permission="zope.Public"
- name="+portlet-product-codestatistics-content"
- template="../templates/product-portlet-codestatistics-content.pt"/>
<browser:page
for="lp.registry.interfaces.product.IProduct"
layer="lp.code.publisher.CodeLayer"
=== modified file 'lib/lp/code/templates/product-branches.pt'
--- lib/lp/code/templates/product-branches.pt 2010-09-28 19:25:54 +0000
+++ lib/lp/code/templates/product-branches.pt 2010-10-14 20:29:43 +0000
@@ -49,7 +49,6 @@
tal:content="structure link/render"></p>
</div>
- <div tal:replace="structure context/@@+portlet-product-codestatistics" />
</div>
</metal:side>
@@ -57,6 +56,10 @@
<tal:branch-summary content="structure context/@@+branch-summary" />
+ <tal:code-statistics
+ condition="view/branch_count"
+ replace="structure context/@@+portlet-product-codestatistics" />
+
<tal:has-branches condition="view/branch_count"
define="branches view/branches">
<tal:branchlisting content="structure branches/@@+branch-listing" />
=== removed file 'lib/lp/code/templates/product-portlet-codestatistics-content.pt'
--- lib/lp/code/templates/product-portlet-codestatistics-content.pt 2010-09-28 19:25:54 +0000
+++ lib/lp/code/templates/product-portlet-codestatistics-content.pt 1970-01-01 00:00:00 +0000
@@ -1,55 +0,0 @@
-<tal:portlet-product-codestatistics-content
- xmlns:tal="http://xml.zope.org/namespaces/tal";
- xmlns:metal="http://xml.zope.org/namespaces/metal";>
-
- <tr tal:define="menu context/menu:branches" class="code-links"
- id="merge-counts">
- <td class="code-count"
- tal:define="count menu/active_review_count"
- tal:content="count" />
- <td>
- <tal:link
- define="link menu/active_reviews"
- replace="structure link/render"
- />
- </td>
- </tr>
-
- <tr class="code-links" id="branch-count-summary">
- <td class="code-count"
- tal:define="count view/branch_count"
- tal:content="count" />
- <td>
- <tal:branches replace="view/branch_text">branches</tal:branches
- ><tal:has-branches condition="view/branch_count">
- owned by
- <tal:individuals condition="view/person_owner_count">
- <tal:owners content="view/person_owner_count">42</tal:owners>
- <tal:people replace="view/person_text">people</tal:people
- ></tal:individuals
- ><tal:teams condition="view/team_owner_count">
- <tal:individuals condition="view/person_owner_count">
- and
- </tal:individuals>
- <tal:toc content="view/team_owner_count">1</tal:toc>
- <tal:people replace="view/team_text">team</tal:people
- ></tal:teams></tal:has-branches>
- </td>
- </tr>
-
- <tr class="code-links">
- <td class="code-count"
- tal:define="count view/commit_count"
- tal:content="count" />
- <td>
- <tal:commits replace="view/commit_text">commits</tal:commits>
- <tal:has-committers condition="view/committer_count">
- by
- <tal:cc content="view/committer_count">4</tal:cc>
- <tal:people replace="view/committer_text">people</tal:people>
- </tal:has-committers>
- in the last month
- </td>
- </tr>
-
-</tal:portlet-product-codestatistics-content>
=== modified file 'lib/lp/code/templates/product-portlet-codestatistics.pt'
--- lib/lp/code/templates/product-portlet-codestatistics.pt 2010-09-22 18:54:36 +0000
+++ lib/lp/code/templates/product-portlet-codestatistics.pt 2010-10-14 20:29:43 +0000
@@ -2,10 +2,55 @@
xmlns:tal="http://xml.zope.org/namespaces/tal";
xmlns:metal="http://xml.zope.org/namespaces/metal";
xmlns:i18n="http://xml.zope.org/namespaces/i18n";
- class="portlet" id="portlet-product-codestatistics">
-
- <table class="code-links">
- <tbody id="portlet-product-codestatistics"
- tal:content="structure context/@@+portlet-product-codestatistics-content" />
- </table>
+ id="portlet-product-codestatistics">
+
+ <p>
+ <tal:project replace="context/displayname"/> has
+ <!--reviews-->
+ <span tal:define="count context/menu:branches/active_review_count;
+ link context/menu:branches/active_reviews"
+ id="merge-counts">
+ <tal:active-count replace="count"/>
+ <tal:link replace="structure python: link.render().lower()"/></span>.
+
+ <!--branches-->
+ <span tal:define="count view/branch_count;"
+ id="branch-count-summary">
+ It has
+ <tal:branch-count replace="count"/>
+ <tal:branches replace="python: view.branch_text.lower()">
+ branches
+ </tal:branches>
+ <tal:has-branches condition="view/branch_count">
+ owned by
+ <tal:individuals condition="view/person_owner_count">
+ <tal:owners content="view/person_owner_count">42</tal:owners>
+ <tal:people replace="view/person_text">people</tal:people>
+ </tal:individuals>
+ <tal:teams condition="view/team_owner_count">
+ <tal:individuals condition="view/person_owner_count">
+ and
+ </tal:individuals>
+ <tal:toc content="view/team_owner_count">1</tal:toc>
+ <tal:people replace="view/team_text">team</tal:people>
+ </tal:teams>
+ </tal:has-branches></span>.
+ </p>
+
+ <p>
+ <!--commits-->
+ <span id="commits"
+ tal:define="count view/commit_count">
+ It has had
+ <tal:commit-count replace="count"/>
+ <tal:commits replace="python: view.commit_text.lower()">
+ commits
+ </tal:commits>
+ <tal:has-committers condition="view/committer_count">
+ by
+ <tal:cc content="view/committer_count">4</tal:cc>
+ <tal:people replace="view/committer_text">people</tal:people>
+ </tal:has-committers>
+ in the last month.
+ </span>
</div>
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
