Code review comment for lp:~brendan-donegan/checkbox/bug960087_memory_compare_ram_devices

In checkbox.lib.dmi, you added:

+ @property
+ def size(self):
+ attribute = "%s_size" % self.category.lower()
+ size = self._attributes.get(attribute)
+
+ size = int(size.split()[0]) * 1024
+
+ if size:
+ return size
+
+ return None

First, when the attribute exists, it will always interpret the size in kilobytes since it ignores the type in the split. Instead, you should use the string_to_type function defined in checkbox.lib.conversion.

Second, when the attribute doesn't exist, the split() method will fail horribly on a None value. So, I would suggest you write the size property method like this, remembering to import string_to_type at the beginning of the module of course:

+ @property
+ def size(self):
+ attribute = "%s_size" % self.category.lower()
+ size = self._attributes.get(attribute)
+ if size:
+ size = string_to_type(size)
+
+ return size

Still in checkbox.lib.dmi, you added:

+ @property
+ def form(self):
+ attribute = "%s_form" % self.category.lower()
+ form = self._attributes.get(attribute)
+
+ if form:
+ return form
+
+ return None

This could be expressed more simply as:

+ @property
+ def form(self):
+ attribute = "%s_form" % self.category.lower()
+ return self._attributes.get(attribute)

Other than that, the changes look good. After you make these changes, I'll have another look but I think we'll be good to merge. Thanks!