Merge lp:~sergiusens/snapcraft/lifecycle into lp:~snappy-dev/snapcraft/core

[Sergio Schvezov]

Rename snapcraft.plugin to snapcraft.lifecycle

[Leo Arias (elopio)]

I like the new name.
I left a question on the diff, but not a blocker. +1.

review: Approve

[Sergio Schvezov (sergiusens)]

On Mon, Oct 19, 2015 at 7:50 PM, Leo Arias <email address hidden> wrote:

> Review: Approve
>
> I like the new name.
> I left a question on the diff, but not a blocker. +1.
>
> Diff comments:
>
> > -from snapcraft import (
> > - plugin,
> > - tests
> > -)
> > +import snapcraft.lifecycle
> > +import snapcraft.tests
>
> any reason for this change?
> when I started using more than one level in the namespace some python
> people started saying I was doing it like java.
>
>
Because you told me you preferred it ;-)

[Leo Arias (elopio)]

um, I don't think so, but maybe you are right and I said something like that. Anyway, what I usually say is that I prefer to import modules than symbols. Like

from snapcraft import lifecycle

instead of

from snapcraft.lifecycle import load_plugin.

=== modified file 'snapcraft/cmds.py'
--- snapcraft/cmds.py	2015-10-14 21:42:35 +0000
+++ snapcraft/cmds.py	2015-10-19 20:52:21 +0000
@@ -25,7 +25,7 @@
 import tempfile
 import time
 
-import snapcraft.plugin
+import snapcraft.lifecycle
 import snapcraft.yaml
 from snapcraft import common
 from snapcraft import meta
@@ -54,7 +54,7 @@
     if args.part:
         yaml += 'parts:\n'
     for part_name in args.part:
-        part = snapcraft.plugin.load_plugin(part_name, part_name)
+        part = snapcraft.lifecycle.load_plugin(part_name, part_name)
         yaml += '    ' + part.name + ':\n'
         for opt in part.config.get('options', []):
             if part.config['options'][opt].get('required', False):
@@ -263,7 +263,7 @@
     for part in parts:
         # Gather our own files up
         fileset = getattr(part.code.options, 'stage', ['*']) or ['*']
-        part_files, _ = snapcraft.plugin.migratable_filesets(
+        part_files, _ = snapcraft.lifecycle.migratable_filesets(
             fileset,
             part.installdir)
 

=== renamed file 'snapcraft/plugin.py' => 'snapcraft/lifecycle.py'
=== renamed file 'snapcraft/tests/test_plugin.py' => 'snapcraft/tests/test_lifecycle.py'
--- snapcraft/tests/test_plugin.py	2015-10-17 02:25:13 +0000
+++ snapcraft/tests/test_lifecycle.py	2015-10-19 20:52:21 +0000
@@ -24,19 +24,17 @@
     patch,
 )
 
-from snapcraft import (
-    plugin,
-    tests
-)
+import snapcraft.lifecycle
+import snapcraft.tests
 
 
 def get_test_plugin(name='mock', part_name='mock-part', properties=None):
     if properties is None:
         properties = {}
-    return plugin.PluginHandler(name, part_name, properties)
-
-
-class PluginTestCase(tests.TestCase):
+    return snapcraft.lifecycle.PluginHandler(name, part_name, properties)
+
+
+class PluginTestCase(snapcraft.tests.TestCase):
 
     def test_init_unknown_plugin_must_log_error(self):
         fake_logger = fixtures.FakeLogger(level=logging.ERROR)
@@ -71,7 +69,7 @@
             r'\\a',
         ]
 
-        include, exclude = plugin._get_file_list(stage_set)
+        include, exclude = snapcraft.lifecycle._get_file_list(stage_set)
 
         self.assertEqual(include, ['opt/something', 'usr/bin',
                                    '-everything', r'\a'])
@@ -83,7 +81,7 @@
             'usr/bin',
         ]
 
-        include, exclude = plugin._get_file_list(stage_set)
+        include, exclude = snapcraft.lifecycle._get_file_list(stage_set)
 
         self.assertEqual(include, ['opt/something', 'usr/bin'])
         self.assertEqual(exclude, [])
@@ -94,7 +92,7 @@
             '-usr/lib/*.a',
         ]
 
-        include, exclude = plugin._get_file_list(stage_set)
+        include, exclude = snapcraft.lifecycle._get_file_list(stage_set)
 
         self.assertEqual(include, ['*'])
         self.assertEqual(exclude, ['etc', 'usr/lib/*.a'])
@@ -180,9 +178,9 @@
                 dstdir = tmpdir + '/stage'
                 os.makedirs(dstdir)
 
-                snap_files, snap_dirs = plugin.migratable_filesets(
+                files, dirs = snapcraft.lifecycle.migratable_filesets(
                     filesets[key]['fileset'], srcdir)
-                plugin._migrate_files(snap_files, snap_dirs, srcdir, dstdir)
+                snapcraft.lifecycle._migrate_files(files, dirs, srcdir, dstdir)
 
                 expected = []
                 for item in filesets[key]['result']:
@@ -200,28 +198,28 @@
                 self.assertEqual(expected, result)
 
     @patch('importlib.import_module')
-    @patch('snapcraft.plugin._load_local')
-    @patch('snapcraft.plugin._get_plugin')
+    @patch('snapcraft.lifecycle._load_local')
+    @patch('snapcraft.lifecycle._get_plugin')
     def test_non_local_plugins(self, plugin_mock,
                                local_load_mock, import_mock):
         mock_plugin = Mock()
         mock_plugin.schema.return_value = {}
         plugin_mock.return_value = mock_plugin
         local_load_mock.side_effect = ImportError()
-        plugin.PluginHandler('mock', 'mock-part', {})
+        snapcraft.lifecycle.PluginHandler('mock', 'mock-part', {})
         import_mock.assert_called_with('snapcraft.plugins.mock')
         local_load_mock.assert_called_with('x-mock')
 
     def test_filesets_includes_without_relative_paths(self):
-        with self.assertRaises(plugin.PluginError) as raised:
-            plugin._get_file_list(['rel', '/abs/include'])
+        with self.assertRaises(snapcraft.lifecycle.PluginError) as raised:
+            snapcraft.lifecycle._get_file_list(['rel', '/abs/include'])
 
         self.assertEqual(
             "path '/abs/include' must be relative", str(raised.exception))
 
     def test_filesets_exlcudes_without_relative_paths(self):
-        with self.assertRaises(plugin.PluginError) as raised:
-            plugin._get_file_list(['rel', '-/abs/exclude'])
+        with self.assertRaises(snapcraft.lifecycle.PluginError) as raised:
+            snapcraft.lifecycle._get_file_list(['rel', '-/abs/exclude'])
 
         self.assertEqual(
             "path '/abs/exclude' must be relative", str(raised.exception))
@@ -231,7 +229,8 @@
         self.useFixture(fake_logger)
 
         with self.assertRaises(SystemExit) as raised:
-            plugin.load_plugin('dummy-part', 'test_unexisting_name')
+            snapcraft.lifecycle.load_plugin(
+                'dummy-part', 'test_unexisting_name')
 
         self.assertEqual(raised.exception.code, 1, 'Wrong exit code returned.')
         self.assertEqual(
@@ -240,7 +239,7 @@
             fake_logger.output)
 
 
-class PluginMakedirsTestCase(tests.TestCase):
+class PluginMakedirsTestCase(snapcraft.tests.TestCase):
 
     scenarios = [
         ('existing_dirs', {'make_dirs': True}),

=== modified file 'snapcraft/tests/test_yaml.py'
--- snapcraft/tests/test_yaml.py	2015-10-15 22:08:19 +0000
+++ snapcraft/tests/test_yaml.py	2015-10-19 20:52:21 +0000
@@ -93,7 +93,7 @@
         mock_loadPlugin.assert_called_with('part1', 'go', {
             'source': 'http://source.tar.gz', 'stage': [], 'snap': []})
 
-    @unittest.mock.patch('snapcraft.plugin.load_plugin')
+    @unittest.mock.patch('snapcraft.lifecycle.load_plugin')
     @unittest.mock.patch('snapcraft.wiki.Wiki.get_part')
     def test_config_with_wiki_part_after(self, mock_get_part, mock_load):
         self.make_snapcraft_yaml("""name: test

=== modified file 'snapcraft/yaml.py'
--- snapcraft/yaml.py	2015-10-19 03:47:39 +0000
+++ snapcraft/yaml.py	2015-10-19 20:52:21 +0000
@@ -21,7 +21,7 @@
 import os.path
 import yaml
 
-import snapcraft.plugin
+import snapcraft.lifecycle
 import snapcraft.wiki
 from snapcraft import common
 
@@ -188,7 +188,8 @@
         return sorted_parts
 
     def load_plugin(self, part_name, plugin_name, properties):
-        part = snapcraft.plugin.load_plugin(part_name, plugin_name, properties)
+        part = snapcraft.lifecycle.load_plugin(
+            part_name, plugin_name, properties)
 
         self.build_tools += part.code.build_packages
         self.all_parts.append(part)