diff mbox series

[1/1] scanpypi: write every license file once

Message ID 20191008090404.26700-2-asafka7@gmail.com
State Accepted
Headers show
Series [1/1] scanpypi: write every license file once | expand

Commit Message

Asaf Kahlon Oct. 8, 2019, 9:04 a.m. UTC
On some cases, when the package contains multiple license files
and some of them from the same type, the scanpypi script will write
the same license type more than once under _LICENSE.
Hence, before creating the _LICENSE variable, we'll remove every
possible duplication.

Signed-off-by: Asaf Kahlon <asafka7@gmail.com>
---
 utils/scanpypi | 2 ++
 1 file changed, 2 insertions(+)

Comments

Thomas Petazzoni Oct. 8, 2019, 9:22 p.m. UTC | #1
Hello,

+Yegor in Cc.

On Tue,  8 Oct 2019 12:04:04 +0300
Asaf Kahlon <asafka7@gmail.com> wrote:

> On some cases, when the package contains multiple license files
> and some of them from the same type, the scanpypi script will write
> the same license type more than once under _LICENSE.
> Hence, before creating the _LICENSE variable, we'll remove every
> possible duplication.
> 
> Signed-off-by: Asaf Kahlon <asafka7@gmail.com>
> ---
>  utils/scanpypi | 2 ++
>  1 file changed, 2 insertions(+)

Yegor, it would be nice to have your review on this patch. Thanks!

Thomas
Yegor Yefremov Oct. 9, 2019, 5:36 p.m. UTC | #2
On Tue, Oct 8, 2019 at 11:22 PM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> Hello,
>
> +Yegor in Cc.
>
> On Tue,  8 Oct 2019 12:04:04 +0300
> Asaf Kahlon <asafka7@gmail.com> wrote:
>
> > On some cases, when the package contains multiple license files
> > and some of them from the same type, the scanpypi script will write
> > the same license type more than once under _LICENSE.
> > Hence, before creating the _LICENSE variable, we'll remove every
> > possible duplication.
> >
> > Signed-off-by: Asaf Kahlon <asafka7@gmail.com>
> > ---
> >  utils/scanpypi | 2 ++
> >  1 file changed, 2 insertions(+)
>
> Yegor, it would be nice to have your review on this patch. Thanks!

On Sun, Oct 6, 2019 at 6:33 PM Adam Ford <aford173@gmail.com> wrote:
>
> There are two checks to see if the manual gpio is configured, but
> these the check is seeing if the structure is NULL instead it
> should check to see if there are CTS and/or RTS pins defined.
>
> This patch uses checks for those individual pins instead of
> checking for the structure itself to restore auto RTS/CTS.
>
> Signed-off-by: Adam Ford <aford173@gmail.com>

Reviewed-by: Yegor Yefremov <yegorslists@googlemail.com>
Thomas Petazzoni Oct. 10, 2019, 7:29 a.m. UTC | #3
Hello Yegor,

On Wed, 9 Oct 2019 19:36:23 +0200
Yegor Yefremov <yegorslists@googlemail.com> wrote:

> > There are two checks to see if the manual gpio is configured, but
> > these the check is seeing if the structure is NULL instead it
> > should check to see if there are CTS and/or RTS pins defined.
> >
> > This patch uses checks for those individual pins instead of
> > checking for the structure itself to restore auto RTS/CTS.
> >
> > Signed-off-by: Adam Ford <aford173@gmail.com>  

Is that a bad copy/paste ? It seems like you were working on some
serial port kernel commits at the same time, isn't it ? :-)

> Reviewed-by: Yegor Yefremov <yegorslists@googlemail.com>

Anyway, thanks for the review! :-)

Thomas
Yegor Yefremov Oct. 10, 2019, 8:38 a.m. UTC | #4
Hello Thomas,

On Thu, Oct 10, 2019 at 9:29 AM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> Hello Yegor,
>
> On Wed, 9 Oct 2019 19:36:23 +0200
> Yegor Yefremov <yegorslists@googlemail.com> wrote:
>
> > > There are two checks to see if the manual gpio is configured, but
> > > these the check is seeing if the structure is NULL instead it
> > > should check to see if there are CTS and/or RTS pins defined.
> > >
> > > This patch uses checks for those individual pins instead of
> > > checking for the structure itself to restore auto RTS/CTS.
> > >
> > > Signed-off-by: Adam Ford <aford173@gmail.com>
>
> Is that a bad copy/paste ? It seems like you were working on some
> serial port kernel commits at the same time, isn't it ? :-)

Every time I try to copy/paste something on a laptop I get similar
results. Sorry for that.

Yegor

> > Reviewed-by: Yegor Yefremov <yegorslists@googlemail.com>
>
> Anyway, thanks for the review! :-)
>
> Thomas
> --
> Thomas Petazzoni, CTO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
Arnout Vandecappelle Oct. 11, 2019, 9:12 p.m. UTC | #5
On 08/10/2019 11:04, Asaf Kahlon wrote:
> On some cases, when the package contains multiple license files
> and some of them from the same type, the scanpypi script will write
> the same license type more than once under _LICENSE.
> Hence, before creating the _LICENSE variable, we'll remove every
> possible duplication.
> 
> Signed-off-by: Asaf Kahlon <asafka7@gmail.com>

 Applied to master, thanks.


> ---
>  utils/scanpypi | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/utils/scanpypi b/utils/scanpypi
> index 55b3d1e61c..b48eda49aa 100755
> --- a/utils/scanpypi
> +++ b/utils/scanpypi
> @@ -461,6 +461,7 @@ class BuildrootPackage():
>                        ' likely wrong, please change it if need be'.format(
>                            license=', '.join(licenses)))

 I notice now that there's something wrong here: this is under a condition that
licenses is empty, so this statement makes no sense...

>                  licenses = [self.metadata['info']['license']]
> +            licenses = set(licenses)
>              license_line = '{name}_LICENSE = {license}\n'.format(
>                  name=self.mk_name,
>                  license=', '.join(licenses))
> @@ -473,6 +474,7 @@ class BuildrootPackage():
>                      license_names.append(match.license.id)
>                  else:
>                      license_names.append("FIXME: license id couldn't be detected")
> +            license_names = set(license_names)
>  
>              if len(license_names) > 0:
>                  license_line = ('{name}_LICENSE ='

 We have exactly the same code here (including the line you added) in both
branches of the condition, so I smell a refactoring opportunity!

 Regards,
 Arnout
diff mbox series

Patch

diff --git a/utils/scanpypi b/utils/scanpypi
index 55b3d1e61c..b48eda49aa 100755
--- a/utils/scanpypi
+++ b/utils/scanpypi
@@ -461,6 +461,7 @@  class BuildrootPackage():
                       ' likely wrong, please change it if need be'.format(
                           license=', '.join(licenses)))
                 licenses = [self.metadata['info']['license']]
+            licenses = set(licenses)
             license_line = '{name}_LICENSE = {license}\n'.format(
                 name=self.mk_name,
                 license=', '.join(licenses))
@@ -473,6 +474,7 @@  class BuildrootPackage():
                     license_names.append(match.license.id)
                 else:
                     license_names.append("FIXME: license id couldn't be detected")
+            license_names = set(license_names)
 
             if len(license_names) > 0:
                 license_line = ('{name}_LICENSE ='