Merge lp:~mordred/glance/conditional-sphinx into lp:~hudson-openstack/glance/trunk

Proposed by Monty Taylor
Status: Merged
Approved by: Monty Taylor
Approved revision: 52
Merged at revision: 53
Proposed branch: lp:~mordred/glance/conditional-sphinx
Merge into: lp:~hudson-openstack/glance/trunk
Diff against target: 47 lines (+15/-11)
1 file modified
setup.py (+15/-11)
To merge this branch: bzr merge lp:~mordred/glance/conditional-sphinx
Reviewer Review Type Date Requested Status
Rick Harris (community) Needs Fixing
Devin Carlen (community) Approve
Jay Pipes (community) Approve
Review via email: mp+47452@code.launchpad.net

Description of the change

Should fix the sphinx issue.

To post a comment you must log in.
Revision history for this message
Jay Pipes (jaypipes) wrote :

cool.

review: Approve
Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

The person who submitted this branch (mordred) does not appear in http://wiki.openstack.org/Contributors?action=raw

Revision history for this message
Devin Carlen (devcamcar) wrote :

lgtm

review: Approve
Revision history for this message
Rick Harris (rconradharris) wrote :

Small-nit:

36 +except:

The unguarded `except` looks a little suspicious, should this be `except ImportError` or are there other exceptions here that would be considered 'normal'?

review: Needs Fixing
Revision history for this message
Monty Taylor (mordred) wrote :

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 01/25/2011 01:41 PM, Rick Harris wrote:
> Review: Needs Fixing Small-nit:
>
> 36 +except:
>
> The unguarded `except` looks a little suspicious, should this be
> `except ImportError` or are there other exceptions here that would be
> considered 'normal'?

Well - normally I'd agree with you. In _this_ case, the idea is that we
want sphinx building to be optional - so if something goes wrong in
setting that up, we'd like to just move on.

I can put in an ImportError: trap though if you'd prefer.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk0/WhwACgkQ2Jv7/VK1RgEzvACfZN9jwkvA7gv8R9rNvyopNZRt
40IAoOa7C6ZGIANSEkyqIpoopDnAOSjv
=Ewxf
-----END PGP SIGNATURE-----

Revision history for this message
Rick Harris (rconradharris) wrote :

I'm leaving it as `except`, though, it might be preferable to do something like

try:
  <stuff>
except ImportError:
  pass # if it's not there, don't worry about it
except:
  LOG.warn("Strange thing occured, perhaps look into this?")

Revision history for this message
Rick Harris (rconradharris) wrote :

s/I'm leaving it as/I'm fine leaving it as/

Revision history for this message
Jay Pipes (jaypipes) wrote :

I'll fix the except nit later. Gotta get this in to release a new tarball...

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'setup.py'
--- setup.py 2011-01-25 17:02:07 +0000
+++ setup.py 2011-01-25 19:50:05 +0000
@@ -19,15 +19,6 @@
1919
20from setuptools import setup, find_packages20from setuptools import setup, find_packages
21from setuptools.command.sdist import sdist21from setuptools.command.sdist import sdist
22from sphinx.setup_command import BuildDoc
23
24
25class local_BuildDoc(BuildDoc):
26 def run(self):
27 for builder in ['html', 'man']:
28 self.builder = builder
29 self.finalize_options()
30 BuildDoc.run(self)
3122
3223
33class local_sdist(sdist):24class local_sdist(sdist):
@@ -44,12 +35,25 @@
44 changelog_file.write(changelog)35 changelog_file.write(changelog)
45 sdist.run(self)36 sdist.run(self)
4637
38cmdclass = {'sdist': local_sdist}
39
40try:
41 from sphinx.setup_command import BuildDoc
42 class local_BuildDoc(BuildDoc):
43 def run(self):
44 for builder in ['html', 'man']:
45 self.builder = builder
46 self.finalize_options()
47 BuildDoc.run(self)
48 cmdclass['build_sphinx'] = local_BuildDoc
49
50except:
51 pass
52
4753
48name = 'glance'54name = 'glance'
49version = '0.1.5'55version = '0.1.5'
5056
51cmdclass = {'sdist': local_sdist,
52 'build_sphinx': local_BuildDoc}
5357
54setup(58setup(
55 name=name,59 name=name,

Subscribers

People subscribed via source and target branches