Merge lp:~mnordhoff/loggerhead/config_app_stuff into lp:loggerhead

[Matt Nordhoff]

[Matt Nordhoff (mnordhoff)]

Good morning,

Some improvements to loggerhead.apps.config.Project's interaction with BranchWSGIApp:

* Use BranchWSGIApp.served_url instead of inventing a 'branch_url' attribute.

* Don't set so many attributes on the BranchWSGIApp object, just 'description' and 'name'. (The constructor can handle the rest.)

* Remove '_src_folder' and '_view_config' from view_data_by_name as they were unused.

* Fix a latent semi-bug: it called BranchWSGIApp(friendly_name=view_name), then set BranchWSGIApp.friendly_name to friendly_name. (I decided to go with friendly_name.)

This was inspired by my investigation into bug #375948 (the branch_url method got clobbered), although I already fixed it a different way (just by renaming it).

Mostly this isn't very important, but I feel that reducing the number of attributes being set is safer, plus I did a little cleanup.

You should sanity-check my decision regarding served_url. This means it'll fall back to public_branch if it's set, but that seems okay to me. Aside from that, I think it's fine. Probably. ;D

So...yeah. :)

[Martin Albisetti (beuno)]

Merge! :)
Thanks Matt.

review: Approve

=== modified file 'loggerhead/apps/config.py'
--- loggerhead/apps/config.py	2008-12-15 21:23:24 +0000
+++ loggerhead/apps/config.py	2009-05-13 13:14:04 +0000
@@ -108,20 +108,17 @@
                 friendly_name = history.get_config().get_nickname()
                 if friendly_name is None:
                     friendly_name = view_name
+            branch_url = self._get_branch_url(view, view_config, view_name)
+            description = self._get_description(view, view_config, history)
             self.view_data_by_name[view_name] = {
                 'branch_path': folder,
-                'args': (view_name, view_config, self.graph_cache),
-                'description': self._get_description(view,
-                                                     view_config,
-                                                     history),
-                '_src_folder': folder,
-                '_view_config': view_config,
+                'config': view_config,
+                'description': description,
                 'friendly_name': friendly_name,
+                'graph_cache': self.graph_cache,
                 'name': view_name,
+                'served_url': branch_url,
                 }
-            branch_url = self._get_branch_url(view, view_config, view_name)
-            if branch_url is not None:
-                self.view_data_by_name[view_name]['branch_url'] = branch_url
             self.view_names.append(view_name)
         finally:
             b.unlock()
@@ -132,12 +129,13 @@
             return None
         view_data = view_data.copy()
         branch_path = view_data.pop('branch_path')
-        args = view_data.pop('args')
+        description = view_data.pop('description')
+        name = view_data.pop('name')
         b = bzrlib.branch.Branch.open(branch_path)
         b.lock_read()
-        view = BranchWSGIApp(b, *args)
-        for k in view_data:
-            setattr(view, k, view_data[k])
+        view = BranchWSGIApp(b, **view_data)
+        view.description = description
+        view.name = name
         return view
 
     def call(self, environ, start_response):

=== modified file 'loggerhead/templates/browse.pt'
--- loggerhead/templates/browse.pt	2008-10-18 00:23:09 +0000
+++ loggerhead/templates/browse.pt	2009-05-13 13:14:04 +0000
@@ -43,12 +43,12 @@
                   </a>
                 </td>
               </tr>
-              <tr tal:condition="view/branch_url">
+              <tr tal:condition="view/served_url">
                 <td class="name">
                 </td>
                 <td class="description url" colspan="2">
-                  <a tal:attributes="href view/branch_url"
-                     tal:content="view/branch_url" />
+                  <a tal:attributes="href view/served_url"
+                     tal:content="view/served_url" />
                 </td>
               </tr>
             </tal:block>