DPDK patches and discussions
 help / color / mirror / Atom feed
From: Thomas Monjalon <thomas@monjalon.net>
To: Stephen Hemminger <stephen@networkplumber.org>
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v4] devtools: add new SPDX license compliance checker
Date: Fri, 12 Jun 2020 17:42:13 +0200	[thread overview]
Message-ID: <3800667.TsBKUvMEcr@thomas> (raw)
In-Reply-To: <20200612075312.24299ddd@hermes.lan>

12/06/2020 16:53, Stephen Hemminger:
> On Thu, 11 Jun 2020 23:39:55 +0200
> Thomas Monjalon <thomas@monjalon.net> wrote:
> 
> > 24/02/2020 22:01, Stephen Hemminger:
> > > +tmpfile=$(mktemp)  
> > 
> > Please check how other temp files are created in other scripts
> > for consitency.
> > 
> > > +    git grep -L SPDX-License-Identifier -- \
> > > +	':^.git*' ':^.ci/*' ':^.travis.yml' \
> > > +	':^README' ':^MAINTAINERS' ':^VERSION' ':^ABI_VERSION' \
> > > +	':^*/Kbuild' ':^*/README' \
> > > +	':^license/' ':^doc/' ':^config/' ':^buildtools/' \  
> > 
> > I think doc/ should be part of the license check,
> > same for buildtools/.
> > 
> > > +	':^*.cocci' ':^*.abignore' \
> > > +	':^*.def' ':^*.map' ':^*.ini' ':^*.data' ':^*.cfg' ':^*.txt' \
> > > +	> $tmpfile
> > > +
> > > +    errors=0
> > > +    while read -r line
> > > +    do $quiet || echo $line
> > > +       errors=$((errors + 1))  
> > 
> > I'm surprised this works for you.
> > In general, "while" creates a subshell which makes impossible
> > updating a variable.
> > I recommend using "for" with IFS=$'\n'.
> > 
> > > +    done < $tmpfile
> > > +}
> > > +
> > > +check_boilerplate() {
> > > +    if $verbose ; then
> > > +	echo
> > > +	echo "Files with redundant license text"
> > > +	echo "---------------------------------"
> > > +    fi
> > > +
> > > +    git grep -l Redistribution -- \
> > > +	':^license/' ':^/devtools/check-spdx-tag.sh' |
> > > +	while read line
> > > +	do $quiet || echo $line
> > > +	   warnings=$((warnings + 1))
> > > +	done  
> > 
> > Same comment about "while" subshell.
> > 
> > > +
> > > +    warnings=0
> > > +    while read -r line
> > > +    do $quiet || echo $line
> > > +       warnings=$((errors + 1))  
> > 
> > Here too
> > 
> > > +    done < $tmpfile
> > > +}  
> > 
> > [...]
> > > +Each file must begin with a special comment containing the
> > > +`Software Package Data Exchange (SPDX) License Identfier <https://spdx.org/using-spdx-license-identifier>`_.  
> > 
> > Typo: Identifier
> > 
> > > +
> > > +Generally this is the BSD License, except for code granted special exceptions.  
> > 
> > Is a verb missing?
> > 
> > > +The SPDX licences identifier is sufficient, a file should not contain
> > > +an additional text version of the license (boilerplate).  
> 
> Thanks for the feedback.
> Text processing is simpler in python, will rewrite in next version?

I don't see a need for rewrite.
I think addressing the comments is simpler.



  reply	other threads:[~2020-06-12 15:42 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-29 15:59 [dpdk-dev] [PATCH v2] " Stephen Hemminger
2020-02-07 17:39 ` Stephen Hemminger
2020-02-07 17:52 ` [dpdk-dev] [PATCH v3] " Stephen Hemminger
2020-02-22 15:43   ` Thomas Monjalon
2020-02-22 15:45   ` Thomas Monjalon
2020-02-24 21:01 ` [dpdk-dev] [PATCH v4] " Stephen Hemminger
2020-04-28 20:15   ` Stephen Hemminger
2020-06-11 18:46   ` Stephen Hemminger
2020-06-11 21:32     ` Thomas Monjalon
2020-06-11 21:39   ` Thomas Monjalon
2020-06-12  8:36     ` Gaëtan Rivet
2020-06-12 14:53     ` Stephen Hemminger
2020-06-12 15:42       ` Thomas Monjalon [this message]
2020-06-12  9:05   ` Gaëtan Rivet
2020-07-14 23:23     ` Stephen Hemminger
2020-02-26  1:14 ` [dpdk-dev] [PATCH] " Stephen Hemminger
2020-07-14 23:21 ` [dpdk-dev] [PATCH v5] " Stephen Hemminger
2020-07-14 23:25   ` Stephen Hemminger
2020-07-30 22:00     ` Thomas Monjalon
2020-07-23  4:36   ` Stephen Hemminger
2020-07-30 22:06   ` Thomas Monjalon
2020-07-30 23:41     ` Stephen Hemminger
2020-08-26 15:12     ` Stephen Hemminger
2020-08-26 15:43       ` Bruce Richardson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3800667.TsBKUvMEcr@thomas \
    --to=thomas@monjalon.net \
    --cc=dev@dpdk.org \
    --cc=stephen@networkplumber.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).