loggerhead

Merge lp://staging/~mnordhoff/loggerhead/one-config into lp://staging/loggerhead

  1. one-config
  2. Merge into trunk-rich
Proposed by Matt Nordhoff
Status: Merged
Merged at revision: not available
Proposed branch: lp://staging/~mnordhoff/loggerhead/one-config
Merge into: lp://staging/loggerhead
Diff against target: None lines
To merge this branch: bzr merge lp://staging/~mnordhoff/loggerhead/one-config
Reviewer Review Type Date Requested Status
Michael Hudson-Doyle Approve
Review via email: mp+5643@code.staging.launchpad.net
To post a comment you must log in.
Revision history for this message
Matt Nordhoff (mnordhoff) wrote :

Hello!

This makes serve-branches pass its LoggerheadConfig object to {User,}BranchesFromFileSystemRoot, which then passes it to BranchesFromFileSystemServer instead of creating a new one every time.

Um. I honestly don't know if this is a benefit in any way, but it seems like a good idea to me. There's no need to call optparse more than once, so why do it? I think it's worth a little complexity.

The biggest open issue, IMO, is whether to make the "config" arguments optional, for the sake of backwards compatibility. I did it this way since it was the least typing, but I don't really have a preference.

I also made only the minimal code changes necessary. It would probably be better to restructure things, but again, less typing. :D

(This change benefits me because it makes it possible to hack the LoggerheadConfig object in serve-branches to do weird things instead of having to manipulate sys.argv or something.)

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

This looks like a step in the right direction -- I'd like to go further -- but it's good enough for now.

review: Approve

Updating diff...

An updated diff will be available in a few minutes. Reload to see the changes.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'loggerhead/apps/filesystem.py'
2--- loggerhead/apps/filesystem.py 2009-04-07 18:29:10 +0000
3+++ loggerhead/apps/filesystem.py 2009-04-17 00:14:16 +0000
4@@ -20,7 +20,7 @@
5 self.path = path
6 self.root = root
7 self.name = name
8- self._config = LoggerheadConfig()
9+ self._config = root._config
10
11 def app_for_branch(self, branch):
12 if not self.name:
13@@ -70,9 +70,10 @@
14
15 class BranchesFromFileSystemRoot(object):
16
17- def __init__(self, folder):
18+ def __init__(self, folder, config):
19 self.graph_cache = lru_cache.LRUCache()
20 self.folder = folder
21+ self._config = config
22
23 def __call__(self, environ, start_response):
24 environ['loggerhead.static.url'] = environ['SCRIPT_NAME']
25@@ -92,10 +93,11 @@
26
27 class UserBranchesFromFileSystemRoot(object):
28
29- def __init__(self, folder, trunk_dir):
30+ def __init__(self, folder, config):
31 self.graph_cache = lru_cache.LRUCache()
32 self.folder = folder
33- self.trunk_dir = trunk_dir
34+ self._config = config
35+ self.trunk_dir = config.get_option('trunk_dir')
36
37 def __call__(self, environ, start_response):
38 environ['loggerhead.static.url'] = environ['SCRIPT_NAME']
39
40=== modified file 'serve-branches'
41--- serve-branches 2009-04-16 21:49:48 +0000
42+++ serve-branches 2009-04-17 00:14:16 +0000
43@@ -66,10 +66,9 @@
44 if not config.get_option('trunk_dir'):
45 print "You didn't specify a directory for the trunk directories."
46 sys.exit(1)
47- app = UserBranchesFromFileSystemRoot(
48- path, config.get_option('trunk_dir'))
49+ app = UserBranchesFromFileSystemRoot(path, config)
50 else:
51- app = BranchesFromFileSystemRoot(path)
52+ app = BranchesFromFileSystemRoot(path, config)
53
54 # setup_logging()
55 logging.basicConfig()

Subscribers

People subscribed via source and target branches