[v3,02/12] support/scripts/pkg-stats: store patch files for the package
diff mbox series

Message ID 20200222085715.23769-3-heiko.thiery@gmail.com
State Superseded
Headers show
Series
  • pkg-stats json output improvements
Related show

Commit Message

Heiko Thiery Feb. 22, 2020, 8:57 a.m. UTC
From: Heiko Thiery <heiko.thiery@kontron.com>

Signed-off-by: Heiko Thiery <heiko.thiery@kontron.com>
Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>
---
 support/scripts/pkg-stats | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Titouan Christophe Feb. 23, 2020, 1:35 p.m. UTC | #1
Heiko,


On 2/22/20 9:57 AM, Heiko Thiery wrote:
> From: Heiko Thiery <heiko.thiery@kontron.com>
> 
> Signed-off-by: Heiko Thiery <heiko.thiery@kontron.com>
> Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>

Same as the previous patch, signed off with 2 different email addresses.

> ---
>   support/scripts/pkg-stats | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/support/scripts/pkg-stats b/support/scripts/pkg-stats
> index 8cc78f2f66..4c963cef0f 100755
> --- a/support/scripts/pkg-stats
> +++ b/support/scripts/pkg-stats
> @@ -65,6 +65,7 @@ class Package:
>           self.has_license_files = False
>           self.has_hash = False
>           self.patch_count = 0
> +        self.patch_files = []
>           self.warnings = 0
>           self.current_version = None
>           self.url = None
> @@ -131,10 +132,10 @@ class Package:
>           """
>           Fills in the .patch_count field
>           """
> -        self.patch_count = 0
>           pkgdir = os.path.dirname(self.path)
>           for subdir, _, _ in os.walk(pkgdir):
> -            self.patch_count += len(fnmatch.filter(os.listdir(subdir), '*.patch'))
> +            self.patch_files = fnmatch.filter(os.listdir(subdir), '*.patch')
> +            self.patch_count = len(self.patch_files)

We can compute the patch_count only once after looping through all the 
files, no need to update it at each step.

Maybe we can also entirely remove this patch_count attribute from the 
Package class, and define it as a property:

class Package:
     # ...
     @property
     def patch_count(self):
         return len(self.patch_files)

>   
>       def set_current_version(self):
>           """
> 


Best regards,

Titouan
Heiko Thiery Feb. 23, 2020, 9:23 p.m. UTC | #2
Hi Titouan and all,

Am So., 23. Feb. 2020 um 14:35 Uhr schrieb Titouan Christophe
<titouan.christophe@railnova.eu>:
>
> Heiko,
>
>
> On 2/22/20 9:57 AM, Heiko Thiery wrote:
> > From: Heiko Thiery <heiko.thiery@kontron.com>
> >
> > Signed-off-by: Heiko Thiery <heiko.thiery@kontron.com>
> > Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>
>
> Same as the previous patch, signed off with 2 different email addresses.
>
> > ---
> >   support/scripts/pkg-stats | 5 +++--
> >   1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/support/scripts/pkg-stats b/support/scripts/pkg-stats
> > index 8cc78f2f66..4c963cef0f 100755
> > --- a/support/scripts/pkg-stats
> > +++ b/support/scripts/pkg-stats
> > @@ -65,6 +65,7 @@ class Package:
> >           self.has_license_files = False
> >           self.has_hash = False
> >           self.patch_count = 0
> > +        self.patch_files = []
> >           self.warnings = 0
> >           self.current_version = None
> >           self.url = None
> > @@ -131,10 +132,10 @@ class Package:
> >           """
> >           Fills in the .patch_count field
> >           """
> > -        self.patch_count = 0
> >           pkgdir = os.path.dirname(self.path)
> >           for subdir, _, _ in os.walk(pkgdir):
> > -            self.patch_count += len(fnmatch.filter(os.listdir(subdir), '*.patch'))
> > +            self.patch_files = fnmatch.filter(os.listdir(subdir), '*.patch')
> > +            self.patch_count = len(self.patch_files)
>
> We can compute the patch_count only once after looping through all the
> files, no need to update it at each step.
>
> Maybe we can also entirely remove this patch_count attribute from the
> Package class, and define it as a property:

This is a good idea. Just to mention then we lose this field in the
json output. I don't know if there is another user for that. I can
change the fields in the buildroot-stats app to use the length of the
patches list. Has anyone concerns about remove the attribute?

> class Package:
>      # ...
>      @property
>      def patch_count(self):
>          return len(self.patch_files)
>
> >
> >       def set_current_version(self):
> >           """
> >
>
>
> Best regards,
>
> Titouan

Thank you,
Heiko

Patch
diff mbox series

diff --git a/support/scripts/pkg-stats b/support/scripts/pkg-stats
index 8cc78f2f66..4c963cef0f 100755
--- a/support/scripts/pkg-stats
+++ b/support/scripts/pkg-stats
@@ -65,6 +65,7 @@  class Package:
         self.has_license_files = False
         self.has_hash = False
         self.patch_count = 0
+        self.patch_files = []
         self.warnings = 0
         self.current_version = None
         self.url = None
@@ -131,10 +132,10 @@  class Package:
         """
         Fills in the .patch_count field
         """
-        self.patch_count = 0
         pkgdir = os.path.dirname(self.path)
         for subdir, _, _ in os.walk(pkgdir):
-            self.patch_count += len(fnmatch.filter(os.listdir(subdir), '*.patch'))
+            self.patch_files = fnmatch.filter(os.listdir(subdir), '*.patch')
+            self.patch_count = len(self.patch_files)
 
     def set_current_version(self):
         """