Merge lp:~bac/tarmac/make_treedir into lp:tarmac

Proposed by Brad Crittenden
Status: Merged
Approved by: Paul Hummer
Approved revision: 401
Merged at revision: 400
Proposed branch: lp:~bac/tarmac/make_treedir
Merge into: lp:tarmac
Diff against target: 233 lines (+64/-24)
8 files modified
HACKING (+11/-0)
docs/introduction.txt (+23/-18)
docs/writingplugins.txt (+1/-1)
tarmac/bin/commands.py (+2/-2)
tarmac/branch.py (+5/-0)
tarmac/config.py (+2/-2)
tarmac/tests/test_branch.py (+19/-0)
tarmac/tests/test_commands.py (+1/-1)
To merge this branch: bzr merge lp:~bac/tarmac/make_treedir
Reviewer Review Type Date Requested Status
Paul Hummer Approve
Review via email: mp+112840@code.launchpad.net

Commit message

If the dirname() portion of tree_dir does not exist, then create it in create_tree.

Description of the change

If the dirname() portion of tree_dir does not exist, then create it in create_tree.

Also made some changes to the documentation that I think helps clarify things. One change that may be controversial is to *not* use 'tarmac' as the example when explaining the configuration as it is too subtle for new users to see the difference between the [Tarmac] stanzas and the subsequent [lp:tarmac] parts. Perhaps just an introductory statement would've sufficed but it seemed reasonable to avoid the confusion all together.

Some line changes are due to deleting trailing whitespace, which my editor does automatically. Sorry for polluting the diff with those. Similarly, I removed some apostrophes (single-quotes) contained within docstrings as they appear as unbalanced quotes.

To post a comment you must log in.
Revision history for this message
Paul Hummer (rockstar) :
review: Approve
Revision history for this message
Brad Crittenden (bac) wrote :

Thanks for the review Paul. I assume you'll merge it into trunk shortly?

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'HACKING'
2--- HACKING 2010-11-04 16:34:30 +0000
3+++ HACKING 2012-06-29 20:15:23 +0000
4@@ -24,6 +24,17 @@
5
6
7 =============
8+Running Tests
9+=============
10+
11+Tests are run using 'trial' from Twisted and rely on bzrlib.tests. So
12+you must install the `python-twisted-core` and `python-bzrlib.tests`
13+packages in order to run the tests. Once done, from the top of the
14+repository you can run:
15+
16+% trial tarmac
17+
18+=============
19 Writing Tests
20 =============
21
22
23=== modified file 'docs/introduction.txt'
24--- docs/introduction.txt 2011-06-22 09:50:42 +0000
25+++ docs/introduction.txt 2012-06-29 20:15:23 +0000
26@@ -25,13 +25,13 @@
27 =============
28
29 Tarmac gets it's configuration data from ``~/.config/tarmac/tarmac.conf`` --
30-The configuration is split up by projects. As an example, Tarmac will
31-demonstrate its own config throughout this project. To start with, the Tarmac
32-project should be added to the config (with the file being created if it
33-doesn't exist already).::
34+The configuration is split up by projects. As an example, a fictional
35+`phoo` project will be used. The config file starts with items that
36+apply to the Tarmac command. Add to the following to the config file,
37+creating it if it doesn't exist already).::
38
39 [Tarmac]
40- # Configuration for the Tarmac project
41+ # Configuration for the Tarmac command.
42
43 This doesn't mean much by itself. Optionally, we can also specify where to
44 write the log (by default, it logs to ~/tarmac.log). A basic log file setting
45@@ -40,19 +40,22 @@
46 [Tarmac]
47 log_file = /var/log/tarmac.log
48
49-Now let's provide some branches for Tarmac to manage. How about Tarmac's
50+If placing the file in ``/var/log`` be sure the user running
51+``tarmac`` has write permission to that file.
52+
53+Now let's provide some branches for Tarmac to manage. How about phoo's
54 development focus first? Specifying this is REALLY easy. Now my config file
55 will look like this::
56
57 [Tarmac]
58 log_file = /var/log/tarmac.log
59
60- [lp:tarmac]
61+ [lp:phoo]
62
63 That's it! I specify the branch by it's lp: name, and Tarmac knows that when
64-it's instructed to land branches, it should check up on lp:tarmac for approved
65+it's instructed to land branches, it should check up on lp:phoo for approved
66 merge proposals. However, this means that Tarmac has to make a new tree of
67-lp:tarmac every time it makes a landing run. This is rather inefficient. We
68+lp:phoo every time it makes a landing run. This is rather inefficient. We
69 can actually have Tarmac cache a tree, and just update it on every run. This
70 path to the cache tree can be specified by changing the config file to look
71 like this::
72@@ -60,8 +63,8 @@
73 [Tarmac]
74 log_file = /var/log/tarmac.log
75
76- [lp:tarmac]
77- tree_dir = /var/cache/tarmac
78+ [lp:phoo]
79+ tree_dir = /var/cache/tarmac/phoo/trunk
80
81 If this directory or tree doesn't exist, Tarmac will go ahead and create it.
82
83@@ -71,14 +74,15 @@
84
85 Once Tarmac is all configured, simply run ``tarmac merge``. Voila! If there
86 are issues at all, you can run ``tarmac merge --debug`` to get more debug
87-imformation.
88+information.
89
90 ==============
91 Tarmac on Cron
92 ==============
93
94-Tarmac runs best on a cron, because it's doing stuff you shouldn't even need to
95-worry about. To make Tarmac run on a cron, add this to your crontab::
96+Tarmac runs best on a cron job, because it's doing stuff you shouldn't
97+even need to worry about. To make Tarmac run on a cron, add this to
98+your crontab::
99
100 0 * * * * tarmac merge
101
102@@ -190,10 +194,11 @@
103 CIA.vc Notifier
104 ===============
105
106-The CIA.vc Notifier can be configured to notify the CIA.vc service of
107-successful merges. Tarmac has already been configured to work with the CIA.vc
108-service and is set up to notify those in the #tarmac IRC channel on Freenode.
109-This configuration looks like this::
110+The CIA.vc Notifier (see ``http://cia.vc``) can be configured to
111+notify the CIA.vc service of successful merges. Tarmac has already
112+been configured to work with the CIA.vc service and is set up to
113+notify those in the #tarmac IRC channel on Freenode. This
114+configuration looks like this::
115
116 [lp:tarmac]
117 cia_project = tarmac
118
119=== modified file 'docs/writingplugins.txt'
120--- docs/writingplugins.txt 2010-09-17 21:57:28 +0000
121+++ docs/writingplugins.txt 2012-06-29 20:15:23 +0000
122@@ -53,7 +53,7 @@
123 arguments to the plugin are explained below.
124
125 **command**
126- The tarmac command. For instance, a ``tarmac land`` call.
127+ The tarmac command. For instance, a ``tarmac merge`` call.
128
129 **target**
130 An instance of ``tarmac.branch.Branch`` containing details about the target
131
132=== modified file 'tarmac/bin/commands.py'
133--- tarmac/bin/commands.py 2012-06-28 13:50:56 +0000
134+++ tarmac/bin/commands.py 2012-06-29 20:15:23 +0000
135@@ -40,7 +40,7 @@
136 self.config.set('Tarmac', name, False)
137
138 def _usage(self):
139- """Custom _usage for referencing "tarmac" instead of "bzr."""
140+ """Custom _usage for referencing 'tarmac' instead of 'bzr'."""
141 s = 'tarmac ' + self.name() + ' '
142 for aname in self.takes_args:
143 aname = aname.upper()
144@@ -306,7 +306,7 @@
145 "'Approved'".format(entry.queue_status))
146 continue
147
148- if (not self.config.imply_commit_message and
149+ if (not self.config.imply_commit_message and
150 not entry.commit_message):
151 self.logger.debug(
152 " Skipping proposal: proposal has no commit message")
153
154=== modified file 'tarmac/branch.py'
155--- tarmac/branch.py 2010-10-22 17:59:26 +0000
156+++ tarmac/branch.py 2012-06-29 20:15:23 +0000
157@@ -72,6 +72,11 @@
158 self.tree = WorkingTree.open(self.config.tree_dir)
159 else:
160 self.logger.debug('Tree does not exist. Creating dir')
161+ # Create the path up to but not including tree_dir if it does
162+ # not exist.
163+ parent_dir = os.path.dirname(self.config.tree_dir)
164+ if not os.path.exists(parent_dir):
165+ os.makedirs(parent_dir)
166 self.tree = self.bzr_branch.create_checkout(
167 self.config.tree_dir, lightweight=True)
168 except AttributeError:
169
170=== modified file 'tarmac/config.py'
171--- tarmac/config.py 2011-01-05 17:25:58 +0000
172+++ tarmac/config.py 2012-06-29 20:15:23 +0000
173@@ -104,7 +104,7 @@
174 section.startswith('lp:')]
175
176 def _check_config_dirs(self):
177- '''Create the configuration directory if it doesn't exist.'''
178+ '''Create the configuration directory if it does not exist.'''
179 if not os.path.exists(self.CONFIG_HOME):
180 os.makedirs(self.CONFIG_HOME)
181 if not os.path.exists(self.CACHE_HOME):
182@@ -117,7 +117,7 @@
183 class BranchConfig:
184 '''A Branch specific config.
185
186- Instead of providing the whole config for branches, it's better to provide
187+ Instead of providing the whole config for branches, it is better to provide
188 it with only its specific config vars.
189 '''
190
191
192=== modified file 'tarmac/tests/test_branch.py'
193--- tarmac/tests/test_branch.py 2010-10-22 13:51:39 +0000
194+++ tarmac/tests/test_branch.py 2012-06-29 20:15:23 +0000
195@@ -41,6 +41,25 @@
196 self.assertFalse(hasattr(a_branch, 'tree'))
197 self.remove_branch_config(tree_dir)
198
199+ def test_create_missing_parent_dir(self):
200+ '''Test the creation of a TarmacBranch instance in a path that does
201+ not fully exist, with a tree'''
202+ branch_name = 'test_branch'
203+ parent_dir = os.path.join(self.TEST_ROOT, 'missing')
204+ tree_dir = os.path.join(parent_dir, branch_name)
205+ self.add_branch_config(tree_dir)
206+ # Create the mock somewhere other than where the tarmac branch will be
207+ # located. Keep it right under TEST_ROOT so the
208+ # TarmacDirectoryFactory mocking will work.
209+ mock = MockLPBranch(os.path.join(self.TEST_ROOT, branch_name))
210+ self.assertFalse(os.path.exists(parent_dir))
211+ a_branch = branch.Branch.create(mock, self.config, create_tree=True)
212+ self.assertTrue(os.path.exists(parent_dir))
213+ self.assertTrue(isinstance(a_branch, branch.Branch))
214+ self.assertTrue(a_branch.lp_branch.bzr_identity is not None)
215+ self.assertTrue(hasattr(a_branch, 'tree'))
216+ self.remove_branch_config(tree_dir)
217+
218 def test_create_with_tree(self):
219 '''Test the creation of a TarmacBranch with a created tree.'''
220 self.assertTrue(isinstance(self.branch1, branch.Branch))
221
222=== modified file 'tarmac/tests/test_commands.py'
223--- tarmac/tests/test_commands.py 2010-10-12 19:58:22 +0000
224+++ tarmac/tests/test_commands.py 2012-06-29 20:15:23 +0000
225@@ -60,7 +60,7 @@
226 # sys.stdout = old_stdout
227
228 def test_run_already_authenticated(self):
229- '''If the user has already been authenticated, don't try again.'''
230+ '''If the user has already been authenticated, do not try again.'''
231 registry = CommandRegistry(config=self.config)
232 registry.register_command('authenticate', commands.cmd_authenticate)
233 command = registry._get_command(commands.cmd_authenticate,

Subscribers

People subscribed via source and target branches