Hi Curtis, Thanks for the review. I have merged in Brad's branch and pointed the "Configure product branch" links at +setbranch instead of +linkbranch, and I have responded to your comments below. -Edwin > Hi Edwin. > > I have a couple suggestion and questions that I think will take a few minutes > to resolve. > > Using the UI: > > * "bug tracker" is two words. > * Maybe "Configure support tracking" for answers. > * We are asking users to Configure project development branch? I guess > length is the issue here. Fixed. > > Code: > > I see these warnings: > > == Pyflakes notices == > > lib/lp/registry/browser/pillar.py > 28: 'IProductSeries' imported but unused > > lib/lp/registry/browser/product.py > 9: undefined name 'ProductInvolvementView' in __all__ > TODO: lint > > > === modified file 'lib/lp/bugs/templates/bugtarget-bugs.pt' > > --- lib/lp/bugs/templates/bugtarget-bugs.pt 2010-03-04 15:09:10 +0000 > > +++ lib/lp/bugs/templates/bugtarget-bugs.pt 2010-03-19 16:20:16 +0000 > > @@ -169,9 +169,14 @@ > > bug tracking.

> > > >

> - id="no-malone-edit"> > > - Enable > > - bug tracking. > > + id="no-malone-edit" > > + tal:define ="configure_bugtracker > context/menu:overview/configure_bugtracker | nothing" > > + > > > + > + tal:replace="structure configure_bugtracker/fmt:link"/> > > + > + tal:attributes="href string:${context/fmt:url/+edit}"> > > + Enable bug tracking. > > Why isn't this using the context's overview menu edit link that has > permission checking for rendering. > > context/menu:overview/edit:fmt:url > > The language looks wrong. We are interested in Configuring, not enabling. > I think this is for the bugs root page which may be fine in this case. I was afraid of breaking a bunch more tests if I changed the name of the link from "Enable bug tracking", not to mention that there may be push back on the "Configure bug tracker" link later on. > > === modified file 'lib/lp/code/stories/branches/xx-product-branches.txt' > > --- lib/lp/code/stories/branches/xx-product-branches.txt 2010-01-08 > 14:22:42 +0000 > > +++ lib/lp/code/stories/branches/xx-product-branches.txt 2010-03-19 > 16:20:16 +0000 > > @@ -170,7 +170,7 @@ > > the 'Import your project' button is not shown. > > > > >>> admin_browser.open('http://launchpad.dev/gnome-terminal') > > - >>> admin_browser.getLink('Change details').click() > > + >>> admin_browser.getLink('Configure Launchpad Branches').click() > > >>> admin_browser.getControl( > > ... 'Code for this project is published in Bazaar branches ' > > ... 'on Launchpad').click() > > I do not like the use of "Launchpad" in these links. They imply the project > is setting up hosting when we are really interested in gathering project info. > > I have dealt with several angry, confused users who see Launchpad masquerading > or is forking project. I think taking "Launchpad" out of the link will make > it clear we want the know where the upstream branch is. Removed "Launchpad" from the link text. > > === modified file 'lib/lp/registry/browser/pillar.py' > > --- lib/lp/registry/browser/pillar.py 2010-02-17 11:19:42 +0000 > > +++ lib/lp/registry/browser/pillar.py 2010-03-19 16:20:16 +0000 > > > @@ -128,6 +130,78 @@ > > link for link in menuapi.navigation.values() if link.enabled], > > key=attrgetter('sort_key')) > > > > + @cachedproperty > > + def visible_disabled_links(self): > > + """Important disabled links. > > + > > + These are displayed to notify the user to provide configuration > > + info to enable the links. > > + > > + Override the visible_disabled_link_names attribute to change > > + the results. > > + """ > > + involved_menu = MenuAPI(self).navigation > > + important_links = [ > > + involved_menu[name] > > + for name in self.visible_disabled_link_names] > > + return sorted([ > > + link for link in important_links if not link.enabled], > > + key=attrgetter('sort_key')) > + > > Are the "[]" required? sorted() accepts a generator which is what the first > argument provides. Yes, it's necessary. It's a python syntax error if the generator expression is not the sole argument. > > +# This class can't be moved into the browser/product.py file, since > > +# the pillar-views.txt test will fail due to the MenuAPI adapter > > +# for PillarView.enabled_links not working. > > +class ProductInvolvementView(PillarView): > > + """Encourage configuration of involvement links for projects.""" > > + > > + has_involvement = True > > + visible_disabled_link_names = ['submit_code'] > > + > > + def __init__(self, context, request): > > + super(ProductInvolvementView, self).__init__(context, request) > > + self.official_codehosting = ( > > + self.context.development_focus.branch is not None) > > + > > + @property > > + def configuration_links(self): > > + """The enabled involvement links.""" > > + overview_menu = MenuAPI(self.context).overview > > + series_menu = MenuAPI(self.context.development_focus).overview > > + configuration_names = [ > > + 'configure_answers', > > + 'configure_bugtracker', > > + 'configure_translations', > > + ] > > + configuration_links = [ > > + overview_menu[name] for name in configuration_names] > > + link_branch = series_menu['link_branch'] > > + link_branch.text = 'Configure project branch' > > + configuration_links.append(link_branch) > > + return sorted([ > > + link for link in configuration_links if link.enabled], > > + key=attrgetter('sort_key')) > > Are the brackets required in this sorted()? Yes. > > === modified file 'lib/lp/registry/browser/product.py' > > --- lib/lp/registry/browser/product.py 2010-03-09 00:46:37 +0000 > > +++ lib/lp/registry/browser/product.py 2010-03-19 16:20:16 +0000 > > > @@ -331,6 +337,30 @@ > > return Link('+edit', text, icon='edit') > > > > @enabled_with_permission('launchpad.Edit') > > + def configure_bugtracker(self): > > + text = 'Configure project bugtracker' > > + summary = 'Allow users to report bugs on this project' > > + return Link('+configure-bugtracker', text, summary, icon='edit') > > Our intent it to know where bugs are tracked, and possibly use launchpad. > The problem we have at this moment is that it is not possible to set the > upstream contact without also enabling launchpad bug tracking...making it > impossible for Ubuntu to who to contact when the project does not use > launchpad bugs. Changed to "Specify where bugs are tracked for this project". > > + @enabled_with_permission('launchpad.Edit') > > + def configure_answers(self): > > + text = 'Configure project answers' > > + summary = 'Allow users to ask questions on this project' > > + return Link('+configure-answers', text, summary, icon='edit') > > Maybe we want to use the generic term "support tracker". Answers was called > Support and questions "tickets" in Launchpad beta. Done. > > + @enabled_with_permission('launchpad.Edit') > > + def configure_blueprints(self): > > + text = 'Configure project blueprints' > > + summary = 'Allow users to submit blueprints on this project' > > + return Link('+configure-blueprints', text, summary, icon='edit') > > Specifications and meetings tracking is what this enables. Changed to "Enable tracking of specifications and meetings". > > === modified file 'lib/lp/registry/stories/product/xx-product-launchpad- > usage.txt' > > --- lib/lp/registry/stories/product/xx-product-launchpad-usage.txt > 2010-01-20 23:10:13 +0000 > > +++ lib/lp/registry/stories/product/xx-product-launchpad-usage.txt > 2010-03-19 16:20:16 +0000 > > @@ -28,20 +28,14 @@ > > >>> registrant_browser = setupBrowser( > > ... auth='Basic