Code review comment for lp:~nuclearbob/utah/bzr-symlink

Revision history for this message
Javier Collado (javier.collado) wrote :

Looks nice. I've tested it and works fine as well.

Please fix the pep8 error in the exception message:
url.py:225:21: E128 continuation line under-indented for visual indent

Aside from that, may I suggest to create a link to the file returned by the
recursive call to `url_argument`?

=== modified file 'utah/url.py'
--- utah/url.py 2012-12-12 20:21:28 +0000
+++ utah/url.py 2012-12-13 08:32:33 +0000
@@ -225,7 +225,9 @@
                     'path in a bazaar branch: {}'.format(url))
             dirname = os.path.dirname(url)
             link_url = os.path.join(dirname, link_path)
- return url_argument(link_url)
+ target_path = url_argument(link_url)
+ os.remove(full_url)
+ os.link(target_path, full_url)
         elif not os.path.isfile(full_url):
             raise ArgumentTypeError("URL doesn't point to a file "
                                     'in a bazaar branch: {}'

Isn't really important, but this will preserve the original filename, so when
we look at the logs, we don't have the feeling that the preseed doesn't look
like the good one (I've used `os.link`, but `os.symlink` should work as well).

review: Needs Fixing

« Back to merge proposal