> This branch is a first step to refactor class HALDevice in
> l/c/l/scripts/hwdbsubmissions.py
> 
...

r=me, just a few small copy-n-paste/ordering issues below.

Thanks for the detailed description! Makes much more sense now :) It's
nice seeing the preparation for the new UDevDevice in a separate small
branch!


> === modified file 'lib/canonical/launchpad/scripts/hwdbsubmissions.py'
> --- lib/canonical/launchpad/scripts/hwdbsubmissions.py	2009-09-29 15:05:09 +0000
> +++ lib/canonical/launchpad/scripts/hwdbsubmissions.py	2009-10-01 07:42:22 +0000
> @@ -7,7 +7,7 @@
>  data and for the community test submissions.
>  """
>  
> -
> +__metaclass__ = type
>  __all__ = [
>             'SubmissionParser',
>             'process_pending_submissions',
> @@ -1235,59 +1235,29 @@
>          root_device.createDBData(submission, None)
>          return True
>  
> -class HALDevice:
> -    """The representation of a HAL device node."""
> -
> -    def __init__(self, id, udi, properties, parser):
> -        """HALDevice constructor.
> -
> -        :param id: The ID of the HAL device in the submission data as
> -            specified in <device id=...>.
> -        :type id: int
> -        :param udi: The UDI of the HAL device.
> -        :type udi: string
> -        :param properties: The HAL properties of the device.
> -        :type properties: dict
> -        :param parser: The parser processing a submission.
> -        :type parser: SubmissionParser
> -        """
> -        self.id = id
> -        self.udi = udi
> -        self.properties = properties
> +
> +class BaseDevice:
> +    """A base class to represent device data from HAL and udev."""

There should be a blank line here? (like all the other classes in this file).

> === modified file 'lib/canonical/launchpad/scripts/tests/test_hwdb_submission_processing.py'
> --- lib/canonical/launchpad/scripts/tests/test_hwdb_submission_processing.py	2009-06-25 05:30:52 +0000
> +++ lib/canonical/launchpad/scripts/tests/test_hwdb_submission_processing.py	2009-10-01 07:42:22 +0000
> @@ -380,6 +380,51 @@
>                           'Unexpected value of HALDevice.parent_udi, '
>                           'when no parent information available.')
>  
> +    def testHALDeviceDeviceId(self):

I'm guessing you're aware that these should be pep8-style names,

https://dev.launchpad.net/TestsStyleGuide#Python Test Cases

but in this case the rest of the file uses camel-case, so for consistency...

> +        """Test of HALDevice.device_id."""
> +        properties = {}
> +        parser = SubmissionParser(self.log)
> +        device = HALDevice(1, '/some/udi/path', properties, parser)
> +        self.assertEqual(
> +            '/some/udi/path', device.device_id,
> +            'Unexpected value of HALDevice.parent_udi')

Should this be 'Unexpected value of HALDevice.device_id (which actually
returns self.udi not parent_udi?)

> +
> +    def testHALDevicePciClass(self):
> +        """Test of HALDevice.pci_class."""
> +        properties = {
> +            'pci.device_class': (1, 'int'),
> +            }
> +        parser = SubmissionParser(self.log)
> +        device = HALDevice(1, '/some/udi/path', properties, parser)
> +        self.assertEqual(
> +            device.pci_class, 1,

Hardly worth mentioning, but the expected value should be first like
your previous test above.

> +            'Unexpected value of HALDevice.device_class.')

Again, HALDevice.pci_class?

> +
> +        properties = {}
> +        parser = SubmissionParser(self.log)
> +        device = HALDevice(1, '/some/udi/path', properties, parser)
> +        self.assertEqual(
> +            device.pci_class, None,

ordering...

> +            'Unexpected value of HALDevice.device_class for Non-PCI device.')

and pic_class?

> +
> +    def testHALDevicePciSubClass(self):
> +        """Test of HALDevice.pci_subclass."""
> +        properties = {
> +            'pci.device_subclass': (1, 'int'),
> +            }
> +        parser = SubmissionParser(self.log)
> +        device = HALDevice(1, '/some/udi/path', properties, parser)
> +        self.assertEqual(
> +            device.pci_subclass, 1,

ordering...

> +            'Unexpected value of HALDevice.device_class.')

and pci_subclass?

> +
> +        properties = {}
> +        parser = SubmissionParser(self.log)
> +        device = HALDevice(1, '/some/udi/path', properties, parser)
> +        self.assertEqual(
> +            device.pci_subclass, None,

ordering...

> +            'Unexpected value of HALDevice.device_class for Non-PCI device.')

and pci_subclass?

> +
>      def testHalDeviceRawBus(self):
>          """test of HALDevice.raw_bus."""
>          properties = {

Great! All trivialities :)

Thanks Abel.
-- 
Michael