launchpad-reviewers team mailing list archive

["Jeroen T. Vermeulen" <jtv@xxxxxxxxxxxxx>]

Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/ignore-unused into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jtv/maas/ignore-unused/+merge/109200

We have a few places where we import symbols that we don't actually use in the same module, e.g. because they're only there to be re-exported through __all__.  So far we've been suppressing lint warnings about these by mentioning the unused symbols as stand-alone statements, which is a bit ugly.

Here, as discussed on one occasion with Julian and today with Raphaël as well, I create a dedicated helper that does absolutely nothing.  Passing it a symbol, however, makes lint checkers consider that symbol used.  And presto: no more warning!

This is slightly more explicit than just mentioning something for no apparent reason, and moves some of the documentation need into code.  Code is often a better way to express things than comments are.

There will be conflicts on this branch, due to a migration-prep branch I've also got underway which adds migration notices to nearby lines.  No biggie.


Jeroen
-- 
https://code.launchpad.net/~jtv/maas/ignore-unused/+merge/109200
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/maas/ignore-unused into lp:maas.

=== modified file 'src/maasserver/models/__init__.py'
--- src/maasserver/models/__init__.py	2012-06-07 09:30:56 +0000
+++ src/maasserver/models/__init__.py	2012-06-07 18:05:22 +0000
@@ -71,9 +71,7 @@
 from maasserver.models.filestorage import FileStorage
 from maasserver.models.macaddress import MACAddress
 from maasserver.models.nodegroup import NodeGroup
-
-
-NodeGroup
+from maasserver.utils import ignore_unused
 from maasserver.models.sshkey import SSHKey
 from maasserver.models.timestampedmodel import TimestampedModel
 from maasserver.models.userprofile import UserProfile
@@ -88,6 +86,11 @@
     )
 from provisioningserver.tasks import power_on
 
+
+# Suppress warning about NodeGroup being imported, but only used for
+# export in __all__.
+ignore_unused(NodeGroup)
+
 # Special users internal to MAAS.
 SYSTEM_USERS = [
     # For nodes' access to the metadata API:
@@ -734,8 +737,7 @@
 # 'provisioning' is imported so that it can register its signal handlers early
 # on, before it misses anything.
 from maasserver import provisioning
-# We mention 'provisioning' here to silence lint warnings.
-provisioning
+ignore_unused(provisioning)
 
 from maasserver import messages
-messages
+ignore_unused(messages)

=== modified file 'src/maasserver/utils.py'
--- src/maasserver/utils.py	2012-04-30 16:28:30 +0000
+++ src/maasserver/utils.py	2012-06-07 18:05:22 +0000
@@ -11,10 +11,20 @@
 
 __metaclass__ = type
 __all__ = [
+    'ignore_unused',
     'map_enum',
     ]
 
 
+def ignore_unused(*args):
+    """Suppress warnings about unused variables.
+
+    This function does nothing.  Use it whenever you have deliberately
+    unused symbols: pass them to this function and lint checkers will no
+    longer consider them unused.
+    """
+
+
 def map_enum(enum_class):
     """Map out an enumeration class as a "NAME: value" dict."""
     # Filter out anything that starts with '_', which covers private and