[RFC 2/3] models: Add series-related properties to models

Finucane, Stephen stephen.finucane at intel.com
Tue Apr 12 07:11:22 AEST 2016


On 08 Apr 14:50, Andy Doan wrote:
> On 04/01/2016 11:14 AM, Stephen Finucane wrote:
> >The properties will be used to build series from patches in
> >a follow-on patch.
> >
> >TODO add missing tests
> >
> >Signed-off-by: Doug Anderson <dianders at chromium.org>
> 
> >+    @staticmethod
> >+    def _parse_patch_name(name):
> []
> >+        version = 1
> >+        part_num = 1
> >+        num_parts = 1
> >+        series_tags = []
> >+
> >+        # Work on one tag at a time
> >+        for tag in Patch._raw_patch_tags(name):
> >+            mo = re.match(r'(\d*)/(\d*)', tag)
> >+            if mo:
> >+                part_num = int(mo.group(1))
> >+                num_parts = int(mo.group(2))
> >+                continue
> >+
> >+            mo = re.match(r"[vV](\d*)", tag)
> >+            if mo:
> >+                version = int(mo.group(1))
> >+
> >+            series_tags.append(tag)
> >+
> >+        # Add num_parts to the series tags
> >+        series_tags.append("%d parts" % num_parts)
> >+
> >+        return {
> >+            'version': version,
> >+            'part_num': part_num,
> >+            'num_parts': num_parts
> >+        }
> 
> I don't see point in "series_tags", looks like it can be deleted
> with no side effects.
> 
> >+    @property
> >+    def version(self):
> >+        """Get the version of this patch
> >+
> >+        Returns:
> >+            An integral version number.
> >+        """
> >+        return self._parse_patch_name(self.name)['version']
> >+
> >+    @property
> >+    def num_parts(self):
> >+        """Get the number of parts in the series this patch belongs to.
> >+
> >+        Returns:
> >+            The number of parts in the series.
> >+        """
> >+        return self._parse_patch_name(self.name)['num_parts']
> >+
> >+    @property
> >+    def part_num(self):
> >+        """Get the part number of this patch in its series.
> >+
> >+        Returns:
> >+            The part number of this patch in its series.
> >+        """
> >+        return self._parse_patch_name(self.name)['part_num']
> 
> These seems a little inefficient calling _parse_patch_name times to
> get the three properties. Maybe it should be a single property that
> returns a tuple of the three items? Its hard to tell, because
> there's no code in this series that actually uses this new code.
> Which makes me wonder, is there a point adding this code with no use
> of it?

Good thing this was an RFC. This code was used in later commits (this
series isn't complete), but has been since removed. Patch dropped for
the "real" release, and sorry about it being left in there.

Thanks for the review,
Stephen


More information about the Patchwork mailing list