[v4,1/9] dts: add project tools config

  .gitignore contains standard Python-related files.

Apart from that, add configuration for Python tools used in DTS:
Poetry, dependency and package manager
Black, formatter
Pylama, static analysis
Isort, import sorting

.editorconfig modifies the line length to 88, which is the default Black
uses. It seems to be the best of all worlds. [0]

[0] https://black.readthedocs.io/en/stable/the_black_code_style/current_style.html#line-length

Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu>
Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
---
 dts/.editorconfig  |   7 +
 dts/.gitignore     |  14 ++
 dts/README.md      |  15 ++
 dts/poetry.lock    | 474 +++++++++++++++++++++++++++++++++++++++++++++
 dts/pylama.ini     |   8 +
 dts/pyproject.toml |  43 ++++
 6 files changed, 561 insertions(+)
 create mode 100644 dts/.editorconfig
 create mode 100644 dts/.gitignore
 create mode 100644 dts/README.md
 create mode 100644 dts/poetry.lock
 create mode 100644 dts/pylama.ini
 create mode 100644 dts/pyproject.toml
  

> -----Original Message-----
> From: Juraj Linkeš <juraj.linkes@pantheon.tech>
> Sent: Friday, July 29, 2022 6:56 PM
> To: thomas@monjalon.net; david.marchand@redhat.com; Randles, Ronan
> <ronan.randles@intel.com>; Honnappa.Nagarahalli@arm.com;
> ohilyard@iol.unh.edu; Tu, Lijuan <lijuan.tu@intel.com>
> Cc: dev@dpdk.org; Juraj Linkeš <juraj.linkes@pantheon.tech>
> Subject: [PATCH v4 1/9] dts: add project tools config
> 
> .gitignore contains standard Python-related files.
> 
> Apart from that, add configuration for Python tools used in DTS:
> Poetry, dependency and package manager
> Black, formatter
> Pylama, static analysis
> Isort, import sorting
> 
> .editorconfig modifies the line length to 88, which is the default Black uses. It
> seems to be the best of all worlds. [0]
> 
> [0]
> https://black.readthedocs.io/en/stable/the_black_code_style/current_style.htm
> l#line-length
> 
> Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu>
> Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> ---
>  dts/.editorconfig  |   7 +
>  dts/.gitignore     |  14 ++
>  dts/README.md      |  15 ++
>  dts/poetry.lock    | 474 +++++++++++++++++++++++++++++++++++++++++++++
>  dts/pylama.ini     |   8 +
>  dts/pyproject.toml |  43 ++++
>  6 files changed, 561 insertions(+)
>  create mode 100644 dts/.editorconfig
>  create mode 100644 dts/.gitignore
>  create mode 100644 dts/README.md
>  create mode 100644 dts/poetry.lock
>  create mode 100644 dts/pylama.ini
>  create mode 100644 dts/pyproject.toml
>

Reviewed-by: Lijuan Tu <lijuan.tu@intel.com>

Thanks,
Lijuan
  

On Fri, Jul 29, 2022 at 10:55:42AM +0000, Juraj Linkeš wrote:
> .gitignore contains standard Python-related files.
> 
> Apart from that, add configuration for Python tools used in DTS:
> Poetry, dependency and package manager
> Black, formatter
> Pylama, static analysis
> Isort, import sorting
> 
> .editorconfig modifies the line length to 88, which is the default Black
> uses. It seems to be the best of all worlds. [0]
> 
> [0] https://black.readthedocs.io/en/stable/the_black_code_style/current_style.html#line-length
> 
> Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu>
> Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>

Thanks for the work on this. Some review comments inline below.

/Bruce

> ---
>  dts/.editorconfig  |   7 +
>  dts/.gitignore     |  14 ++
>  dts/README.md      |  15 ++
>  dts/poetry.lock    | 474 +++++++++++++++++++++++++++++++++++++++++++++
>  dts/pylama.ini     |   8 +
>  dts/pyproject.toml |  43 ++++
>  6 files changed, 561 insertions(+)
>  create mode 100644 dts/.editorconfig
>  create mode 100644 dts/.gitignore
>  create mode 100644 dts/README.md
>  create mode 100644 dts/poetry.lock
>  create mode 100644 dts/pylama.ini
>  create mode 100644 dts/pyproject.toml
> 
> diff --git a/dts/.editorconfig b/dts/.editorconfig
> new file mode 100644
> index 0000000000..657f959030
> --- /dev/null
> +++ b/dts/.editorconfig
> @@ -0,0 +1,7 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(c) 2022 PANTHEON.tech s.r.o.
> +# See https://editorconfig.org/ for syntax reference.
> +#
> +
> +[*.py]
> +max_line_length = 88

It seems strange to have two different editorconfig settings in DPDK. Is
there a reason that:
a) we can't use 79, the current DPDK default and recommended length by
   pycodestyle? Or alternatively:
b) change all of DPDK to use the 88 setting?

Also, 88 seems an unusual number. How was it chosen/arrived at?

> diff --git a/dts/.gitignore b/dts/.gitignore
> new file mode 100644
> index 0000000000..9c49935b6f
> --- /dev/null
> +++ b/dts/.gitignore
> @@ -0,0 +1,14 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(c) 2022 PANTHEON.tech s.r.o.
> +#
> +
> +# Byte-compiled / optimized / DLL files
> +__pycache__/
> +*.py[cod]
> +*$py.class
> +
> +# IDE files
> +.idea
> +
> +# DTS results
> +output

I think this should be ok to merge into the main DPDK .gitignore file.

> diff --git a/dts/README.md b/dts/README.md
> new file mode 100644
> index 0000000000..d8f88f97fe
> --- /dev/null
<snip>
> diff --git a/dts/pylama.ini b/dts/pylama.ini
> new file mode 100644
> index 0000000000..23fc709b5a
> --- /dev/null
> +++ b/dts/pylama.ini
> @@ -0,0 +1,8 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(c) 2022 University of New Hampshire
> +#
> +
> +[pylama]
> +format = pylint
> +linters = pep8,pycodestyle,pylint
> +ignore = F0401,C0111,E731,E266,E501,E203

I think it would be good to comment on what these ignored values are, so we
can look to remove them in future, or minimise the list.
From checking the docs, is the below correct?

E203 - whitespace before ‘,’, ‘;’, or ‘:’
E266 - too many leading ‘#’ for block comment
E501 - line too long
E731 - do not assign a lambda expression, use a def
C0111 - Missing %s docstring
F0401 - Unable to import %s

Some of these - particularly the first 2 above - look like they should be
relatively easy to fix and remove the need for ignoring the errors. Are the
standards violations in our DTS code or in some dependencies we import or
code taken from elsewhere?

> diff --git a/dts/pyproject.toml b/dts/pyproject.toml

<Snip for brevity>
  

> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: Wednesday, September 7, 2022 6:17 PM
> To: Juraj Linkeš <juraj.linkes@pantheon.tech>
> Cc: thomas@monjalon.net; david.marchand@redhat.com;
> ronan.randles@intel.com; Honnappa.Nagarahalli@arm.com;
> ohilyard@iol.unh.edu; lijuan.tu@intel.com; dev@dpdk.org
> Subject: Re: [PATCH v4 1/9] dts: add project tools config
> 
> On Fri, Jul 29, 2022 at 10:55:42AM +0000, Juraj Linkeš wrote:
> > .gitignore contains standard Python-related files.
> >
> > Apart from that, add configuration for Python tools used in DTS:
> > Poetry, dependency and package manager Black, formatter Pylama, static
> > analysis Isort, import sorting
> >
> > .editorconfig modifies the line length to 88, which is the default
> > Black uses. It seems to be the best of all worlds. [0]
> >
> > [0]
> > https://black.readthedocs.io/en/stable/the_black_code_style/current_st
> > yle.html#line-length
> >
> > Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu>
> > Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> 
> Thanks for the work on this. Some review comments inline below.
> 
> /Bruce
> 
> > ---
> >  dts/.editorconfig  |   7 +
> >  dts/.gitignore     |  14 ++
> >  dts/README.md      |  15 ++
> >  dts/poetry.lock    | 474
> +++++++++++++++++++++++++++++++++++++++++++++
> >  dts/pylama.ini     |   8 +
> >  dts/pyproject.toml |  43 ++++
> >  6 files changed, 561 insertions(+)
> >  create mode 100644 dts/.editorconfig
> >  create mode 100644 dts/.gitignore
> >  create mode 100644 dts/README.md
> >  create mode 100644 dts/poetry.lock
> >  create mode 100644 dts/pylama.ini
> >  create mode 100644 dts/pyproject.toml
> >
> > diff --git a/dts/.editorconfig b/dts/.editorconfig new file mode
> > 100644 index 0000000000..657f959030
> > --- /dev/null
> > +++ b/dts/.editorconfig
> > @@ -0,0 +1,7 @@
> > +# SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2022
> > +PANTHEON.tech s.r.o.
> > +# See https://editorconfig.org/ for syntax reference.
> > +#
> > +
> > +[*.py]
> > +max_line_length = 88
> 
> It seems strange to have two different editorconfig settings in DPDK. Is there a
> reason that:
> a) we can't use 79, the current DPDK default and recommended length by
>    pycodestyle? Or alternatively:
> b) change all of DPDK to use the 88 setting?
> 
> Also, 88 seems an unusual number. How was it chosen/arrived at?
> 

The commit message contains a link to Black's documentation where they explain it:
https://black.readthedocs.io/en/stable/the_black_code_style/current_style.html#line-length

Let me know what you think about it. I think it's reasonable. I'll move the config to the top level .editorconfig file.

> > diff --git a/dts/.gitignore b/dts/.gitignore new file mode 100644
> > index 0000000000..9c49935b6f
> > --- /dev/null
> > +++ b/dts/.gitignore
> > @@ -0,0 +1,14 @@
> > +# SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2022
> > +PANTHEON.tech s.r.o.
> > +#
> > +
> > +# Byte-compiled / optimized / DLL files __pycache__/ *.py[cod]
> > +*$py.class
> > +
> > +# IDE files
> > +.idea
> > +
> > +# DTS results
> > +output
> 
> I think this should be ok to merge into the main DPDK .gitignore file.
> 

Ok, I'll move it there.
A sidenote - should I add Pantheon to the licence header?

> > diff --git a/dts/README.md b/dts/README.md new file mode 100644 index
> > 0000000000..d8f88f97fe
> > --- /dev/null
> <snip>
> > diff --git a/dts/pylama.ini b/dts/pylama.ini new file mode 100644
> > index 0000000000..23fc709b5a
> > --- /dev/null
> > +++ b/dts/pylama.ini
> > @@ -0,0 +1,8 @@
> > +# SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2022
> > +University of New Hampshire #
> > +
> > +[pylama]
> > +format = pylint
> > +linters = pep8,pycodestyle,pylint
> > +ignore = F0401,C0111,E731,E266,E501,E203
> 
> I think it would be good to comment on what these ignored values are, so we
> can look to remove them in future, or minimise the list.
> From checking the docs, is the below correct?
> 
> E203 - whitespace before ‘,’, ‘;’, or ‘:’
> E266 - too many leading ‘#’ for block comment
> E501 - line too long
> E731 - do not assign a lambda expression, use a def
> C0111 - Missing %s docstring
> F0401 - Unable to import %s
> 
> Some of these - particularly the first 2 above - look like they should be relatively
> easy to fix and remove the need for ignoring the errors. Are the standards
> violations in our DTS code or in some dependencies we import or code taken
> from elsewhere?
> 

I'll let Owen comment on this, he devised the ignorelist. I know that these were chosen when were working with the original DTS code, but now that we're submitting smaller chunks and making bigger changes, we should be able to remove some of these. I think we should leave C0111 and we could easily address the rest (which would require more work on this and future patches), but Owen has a better understanding of this.

> > diff --git a/dts/pyproject.toml b/dts/pyproject.toml
> 
> <Snip for brevity>
>
  

On Fri, Sep 09, 2022 at 01:38:33PM +0000, Juraj Linkeš wrote:
> 
> 
> > -----Original Message-----
> > From: Bruce Richardson <bruce.richardson@intel.com>
> > Sent: Wednesday, September 7, 2022 6:17 PM
> > To: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > Cc: thomas@monjalon.net; david.marchand@redhat.com;
> > ronan.randles@intel.com; Honnappa.Nagarahalli@arm.com;
> > ohilyard@iol.unh.edu; lijuan.tu@intel.com; dev@dpdk.org
> > Subject: Re: [PATCH v4 1/9] dts: add project tools config
> > 
> > On Fri, Jul 29, 2022 at 10:55:42AM +0000, Juraj Linkeš wrote:
> > > .gitignore contains standard Python-related files.
> > >
> > > Apart from that, add configuration for Python tools used in DTS:
> > > Poetry, dependency and package manager Black, formatter Pylama, static
> > > analysis Isort, import sorting
> > >
> > > .editorconfig modifies the line length to 88, which is the default
> > > Black uses. It seems to be the best of all worlds. [0]
> > >
> > > [0]
> > > https://black.readthedocs.io/en/stable/the_black_code_style/current_st
> > > yle.html#line-length
> > >
> > > Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu>
> > > Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > 
> > Thanks for the work on this. Some review comments inline below.
> > 
> > /Bruce
> > 
> > > ---
> > >  dts/.editorconfig  |   7 +
> > >  dts/.gitignore     |  14 ++
> > >  dts/README.md      |  15 ++
> > >  dts/poetry.lock    | 474
> > +++++++++++++++++++++++++++++++++++++++++++++
> > >  dts/pylama.ini     |   8 +
> > >  dts/pyproject.toml |  43 ++++
> > >  6 files changed, 561 insertions(+)
> > >  create mode 100644 dts/.editorconfig
> > >  create mode 100644 dts/.gitignore
> > >  create mode 100644 dts/README.md
> > >  create mode 100644 dts/poetry.lock
> > >  create mode 100644 dts/pylama.ini
> > >  create mode 100644 dts/pyproject.toml
> > >
> > > diff --git a/dts/.editorconfig b/dts/.editorconfig new file mode
> > > 100644 index 0000000000..657f959030
> > > --- /dev/null
> > > +++ b/dts/.editorconfig
> > > @@ -0,0 +1,7 @@
> > > +# SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2022
> > > +PANTHEON.tech s.r.o.
> > > +# See https://editorconfig.org/ for syntax reference.
> > > +#
> > > +
> > > +[*.py]
> > > +max_line_length = 88
> > 
> > It seems strange to have two different editorconfig settings in DPDK. Is there a
> > reason that:
> > a) we can't use 79, the current DPDK default and recommended length by
> >    pycodestyle? Or alternatively:
> > b) change all of DPDK to use the 88 setting?
> > 
> > Also, 88 seems an unusual number. How was it chosen/arrived at?
> > 
> 
> The commit message contains a link to Black's documentation where they explain it:
> https://black.readthedocs.io/en/stable/the_black_code_style/current_style.html#line-length
> 
> Let me know what you think about it. I think it's reasonable. I'll move the config to the top level .editorconfig file.
> 

I have no objection to moving this to the top level, but others may like to
keep our python style as standard. Realistically I see three choices here:

1. Force DTS to conform to existing DPDK python style of 79 characters
2. Allow DTS to use 88 chars but the rest of DPDK to keep with 79 chars
3. Allow all of DPDK to use 88 chars.

Of the 3, I like relaxing the 79/80 char limit so #3 seems best to me as
you suggest. However, I'd wait a few days for a desenting opinion before
I'd do a new patchset revision. :-)

/Bruce
  

> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: Friday, September 9, 2022 3:53 PM
> To: Juraj Linkeš <juraj.linkes@pantheon.tech>
> Cc: thomas@monjalon.net; david.marchand@redhat.com;
> ronan.randles@intel.com; Honnappa.Nagarahalli@arm.com;
> ohilyard@iol.unh.edu; lijuan.tu@intel.com; dev@dpdk.org
> Subject: Re: [PATCH v4 1/9] dts: add project tools config
> 
> On Fri, Sep 09, 2022 at 01:38:33PM +0000, Juraj Linkeš wrote:
> >
> >
> > > -----Original Message-----
> > > From: Bruce Richardson <bruce.richardson@intel.com>
> > > Sent: Wednesday, September 7, 2022 6:17 PM
> > > To: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > > Cc: thomas@monjalon.net; david.marchand@redhat.com;
> > > ronan.randles@intel.com; Honnappa.Nagarahalli@arm.com;
> > > ohilyard@iol.unh.edu; lijuan.tu@intel.com; dev@dpdk.org
> > > Subject: Re: [PATCH v4 1/9] dts: add project tools config
> > >
> > > On Fri, Jul 29, 2022 at 10:55:42AM +0000, Juraj Linkeš wrote:
> > > > .gitignore contains standard Python-related files.
> > > >
> > > > Apart from that, add configuration for Python tools used in DTS:
> > > > Poetry, dependency and package manager Black, formatter Pylama,
> > > > static analysis Isort, import sorting
> > > >
> > > > .editorconfig modifies the line length to 88, which is the default
> > > > Black uses. It seems to be the best of all worlds. [0]
> > > >
> > > > [0]
> > > > https://black.readthedocs.io/en/stable/the_black_code_style/curren
> > > > t_st
> > > > yle.html#line-length
> > > >
> > > > Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu>
> > > > Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > >
> > > Thanks for the work on this. Some review comments inline below.
> > >
> > > /Bruce
> > >
> > > > ---
> > > >  dts/.editorconfig  |   7 +
> > > >  dts/.gitignore     |  14 ++
> > > >  dts/README.md      |  15 ++
> > > >  dts/poetry.lock    | 474
> > > +++++++++++++++++++++++++++++++++++++++++++++
> > > >  dts/pylama.ini     |   8 +
> > > >  dts/pyproject.toml |  43 ++++
> > > >  6 files changed, 561 insertions(+)  create mode 100644
> > > > dts/.editorconfig  create mode 100644 dts/.gitignore  create mode
> > > > 100644 dts/README.md  create mode 100644 dts/poetry.lock  create
> > > > mode 100644 dts/pylama.ini  create mode 100644 dts/pyproject.toml
> > > >
> > > > diff --git a/dts/.editorconfig b/dts/.editorconfig new file mode
> > > > 100644 index 0000000000..657f959030
> > > > --- /dev/null
> > > > +++ b/dts/.editorconfig
> > > > @@ -0,0 +1,7 @@
> > > > +# SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2022
> > > > +PANTHEON.tech s.r.o.
> > > > +# See https://editorconfig.org/ for syntax reference.
> > > > +#
> > > > +
> > > > +[*.py]
> > > > +max_line_length = 88
> > >
> > > It seems strange to have two different editorconfig settings in
> > > DPDK. Is there a reason that:
> > > a) we can't use 79, the current DPDK default and recommended length by
> > >    pycodestyle? Or alternatively:
> > > b) change all of DPDK to use the 88 setting?
> > >
> > > Also, 88 seems an unusual number. How was it chosen/arrived at?
> > >
> >
> > The commit message contains a link to Black's documentation where they
> explain it:
> > https://black.readthedocs.io/en/stable/the_black_code_style/current_st
> > yle.html#line-length
> >
> > Let me know what you think about it. I think it's reasonable. I'll move the
> config to the top level .editorconfig file.
> >
> 
> I have no objection to moving this to the top level, but others may like to keep
> our python style as standard. Realistically I see three choices here:
> 
> 1. Force DTS to conform to existing DPDK python style of 79 characters 2. Allow
> DTS to use 88 chars but the rest of DPDK to keep with 79 chars 3. Allow all of
> DPDK to use 88 chars.
> 
> Of the 3, I like relaxing the 79/80 char limit so #3 seems best to me as you
> suggest. However, I'd wait a few days for a desenting opinion before I'd do a
> new patchset revision. :-)
> 

Ok, I'll wait.

> /Bruce
  

> E203 - whitespace before ‘,’, ‘;’, or ‘:’
> E266 - too many leading ‘#’ for block comment
> E501 - line too long
> E731 - do not assign a lambda expression, use a def
> C0111 - Missing %s docstring
> F0401 - Unable to import %s

E203, E266 and E501 were disabled due to pylama fighting with the
autoformatters, so I decided to let the autoformatters win. I think
that C0111 was suppressed because this set of suppressions was from
mainline DTS and that has a lot of functions without documentation. F0401
is disabled due to dependencies on TRex vendored python libraries,
since those will not be possible to import inside of the container. I don't
remember why E731 is set, but it may be due to the rte flow rule generator
I wrote for mainline DTS, which makes use of lambdas extensively to enable
lazy evaluation, so that DTS doesn't need to keep ~2 billion rules in
memory.



On Fri, Sep 9, 2022 at 10:13 AM Juraj Linkeš <juraj.linkes@pantheon.tech>
wrote:

>
>
> > -----Original Message-----
> > From: Bruce Richardson <bruce.richardson@intel.com>
> > Sent: Friday, September 9, 2022 3:53 PM
> > To: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > Cc: thomas@monjalon.net; david.marchand@redhat.com;
> > ronan.randles@intel.com; Honnappa.Nagarahalli@arm.com;
> > ohilyard@iol.unh.edu; lijuan.tu@intel.com; dev@dpdk.org
> > Subject: Re: [PATCH v4 1/9] dts: add project tools config
> >
> > On Fri, Sep 09, 2022 at 01:38:33PM +0000, Juraj Linkeš wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Bruce Richardson <bruce.richardson@intel.com>
> > > > Sent: Wednesday, September 7, 2022 6:17 PM
> > > > To: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > > > Cc: thomas@monjalon.net; david.marchand@redhat.com;
> > > > ronan.randles@intel.com; Honnappa.Nagarahalli@arm.com;
> > > > ohilyard@iol.unh.edu; lijuan.tu@intel.com; dev@dpdk.org
> > > > Subject: Re: [PATCH v4 1/9] dts: add project tools config
> > > >
> > > > On Fri, Jul 29, 2022 at 10:55:42AM +0000, Juraj Linkeš wrote:
> > > > > .gitignore contains standard Python-related files.
> > > > >
> > > > > Apart from that, add configuration for Python tools used in DTS:
> > > > > Poetry, dependency and package manager Black, formatter Pylama,
> > > > > static analysis Isort, import sorting
> > > > >
> > > > > .editorconfig modifies the line length to 88, which is the default
> > > > > Black uses. It seems to be the best of all worlds. [0]
> > > > >
> > > > > [0]
> > > > > https://black.readthedocs.io/en/stable/the_black_code_style/curren
> > > > > t_st
> > > > > yle.html#line-length
> > > > >
> > > > > Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu>
> > > > > Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > > >
> > > > Thanks for the work on this. Some review comments inline below.
> > > >
> > > > /Bruce
> > > >
> > > > > ---
> > > > >  dts/.editorconfig  |   7 +
> > > > >  dts/.gitignore     |  14 ++
> > > > >  dts/README.md      |  15 ++
> > > > >  dts/poetry.lock    | 474
> > > > +++++++++++++++++++++++++++++++++++++++++++++
> > > > >  dts/pylama.ini     |   8 +
> > > > >  dts/pyproject.toml |  43 ++++
> > > > >  6 files changed, 561 insertions(+)  create mode 100644
> > > > > dts/.editorconfig  create mode 100644 dts/.gitignore  create mode
> > > > > 100644 dts/README.md  create mode 100644 dts/poetry.lock  create
> > > > > mode 100644 dts/pylama.ini  create mode 100644 dts/pyproject.toml
> > > > >
> > > > > diff --git a/dts/.editorconfig b/dts/.editorconfig new file mode
> > > > > 100644 index 0000000000..657f959030
> > > > > --- /dev/null
> > > > > +++ b/dts/.editorconfig
> > > > > @@ -0,0 +1,7 @@
> > > > > +# SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2022
> > > > > +PANTHEON.tech s.r.o.
> > > > > +# See https://editorconfig.org/ for syntax reference.
> > > > > +#
> > > > > +
> > > > > +[*.py]
> > > > > +max_line_length = 88
> > > >
> > > > It seems strange to have two different editorconfig settings in
> > > > DPDK. Is there a reason that:
> > > > a) we can't use 79, the current DPDK default and recommended length
> by
> > > >    pycodestyle? Or alternatively:
> > > > b) change all of DPDK to use the 88 setting?
> > > >
> > > > Also, 88 seems an unusual number. How was it chosen/arrived at?
> > > >
> > >
> > > The commit message contains a link to Black's documentation where they
> > explain it:
> > > https://black.readthedocs.io/en/stable/the_black_code_style/current_st
> > > yle.html#line-length
> > >
> > > Let me know what you think about it. I think it's reasonable. I'll
> move the
> > config to the top level .editorconfig file.
> > >
> >
> > I have no objection to moving this to the top level, but others may like
> to keep
> > our python style as standard. Realistically I see three choices here:
> >
> > 1. Force DTS to conform to existing DPDK python style of 79 characters
> 2. Allow
> > DTS to use 88 chars but the rest of DPDK to keep with 79 chars 3. Allow
> all of
> > DPDK to use 88 chars.
> >
> > Of the 3, I like relaxing the 79/80 char limit so #3 seems best to me as
> you
> > suggest. However, I'd wait a few days for a desenting opinion before I'd
> do a
> > new patchset revision. :-)
> >
>
> Ok, I'll wait.
>
> > /Bruce
>
>
  

On Mon, Sep 12, 2022 at 10:06:03AM -0400, Owen Hilyard wrote:
>    > E203 - whitespace before ‘,’, ‘;’, or ‘:’
>    > E266 - too many leading ‘#’ for block comment
>    > E501 - line too long
>    > E731 - do not assign a lambda expression, use a def
>    > C0111 - Missing %s docstring
>    > F0401 - Unable to import %s
>    E203, E266 and E501 were disabled due to pylama fighting with the
>    autoformatters, so I decided to let the autoformatters win.

That sounds strange. Is there no way to configure the auto-formatters to
avoid these issues? E203 I think we definitely should look to re-enable in
DTS.

> I think
>    that C0111 was suppressed because this set of suppressions was from
>    mainline DTS and that has a lot of functions without
>    documentation.

Is this something we can fix as we go, migrating to DPDK main tree?

>  F0401 is disabled due to dependencies on TRex vendored
>    python libraries, since those will not be possible to import inside of
>    the container.

That seems fair enough to keep disabled.

> I don't remember why E731 is set, but it may be due to
>    the rte flow rule generator I wrote for mainline DTS, which makes use
>    of lambdas extensively to enable lazy evaluation, so that DTS doesn't
>    need to keep ~2 billion rules in memory.

That sounds reasonable too.

Can we perhaps target getting from 6 disabled warnings to 2 or 3 at most?

Other opinions?

/Bruce
  

> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: Monday, September 12, 2022 5:15 PM
> To: Owen Hilyard <ohilyard@iol.unh.edu>
> Cc: Juraj Linkeš <juraj.linkes@pantheon.tech>; thomas@monjalon.net;
> david.marchand@redhat.com; ronan.randles@intel.com;
> Honnappa.Nagarahalli@arm.com; lijuan.tu@intel.com; dev@dpdk.org
> Subject: Re: [PATCH v4 1/9] dts: add project tools config
> 
> On Mon, Sep 12, 2022 at 10:06:03AM -0400, Owen Hilyard wrote:
> >    > E203 - whitespace before ‘,’, ‘;’, or ‘:’
> >    > E266 - too many leading ‘#’ for block comment
> >    > E501 - line too long
> >    > E731 - do not assign a lambda expression, use a def
> >    > C0111 - Missing %s docstring
> >    > F0401 - Unable to import %s
> >    E203, E266 and E501 were disabled due to pylama fighting with the
> >    autoformatters, so I decided to let the autoformatters win.
> 
> That sounds strange. Is there no way to configure the auto-formatters to avoid
> these issues? E203 I think we definitely should look to re-enable in DTS.
> 
> > I think
> >    that C0111 was suppressed because this set of suppressions was from
> >    mainline DTS and that has a lot of functions without
> >    documentation.
> 
> Is this something we can fix as we go, migrating to DPDK main tree?
> 
> >  F0401 is disabled due to dependencies on TRex vendored
> >    python libraries, since those will not be possible to import inside of
> >    the container.
> 
> That seems fair enough to keep disabled.
> 
> > I don't remember why E731 is set, but it may be due to
> >    the rte flow rule generator I wrote for mainline DTS, which makes use
> >    of lambdas extensively to enable lazy evaluation, so that DTS doesn't
> >    need to keep ~2 billion rules in memory.
> 
> That sounds reasonable too.
> 
> Can we perhaps target getting from 6 disabled warnings to 2 or 3 at most?
> 
> Other opinions?
> 

We don't actually need the ignorelist at this point - I've removed the ignorelist, fixed a few issues and the tools don't report any problems. I'd like to start here and disable checks (either globally or in code where it's needed) when we actually need them (in a future patch).

> /Bruce
>
  

On Tue, Sep 13, 2022 at 12:08:48PM +0000, Juraj Linkeš wrote:
> 
> 
> > -----Original Message-----
> > From: Bruce Richardson <bruce.richardson@intel.com>
> > Sent: Monday, September 12, 2022 5:15 PM
> > To: Owen Hilyard <ohilyard@iol.unh.edu>
> > Cc: Juraj Linkeš <juraj.linkes@pantheon.tech>; thomas@monjalon.net;
> > david.marchand@redhat.com; ronan.randles@intel.com;
> > Honnappa.Nagarahalli@arm.com; lijuan.tu@intel.com; dev@dpdk.org
> > Subject: Re: [PATCH v4 1/9] dts: add project tools config
> > 
> > On Mon, Sep 12, 2022 at 10:06:03AM -0400, Owen Hilyard wrote:
> > >    > E203 - whitespace before ‘,’, ‘;’, or ‘:’
> > >    > E266 - too many leading ‘#’ for block comment
> > >    > E501 - line too long
> > >    > E731 - do not assign a lambda expression, use a def
> > >    > C0111 - Missing %s docstring
> > >    > F0401 - Unable to import %s
> > >    E203, E266 and E501 were disabled due to pylama fighting with the
> > >    autoformatters, so I decided to let the autoformatters win.
> > 
> > That sounds strange. Is there no way to configure the auto-formatters to avoid
> > these issues? E203 I think we definitely should look to re-enable in DTS.
> > 
> > > I think
> > >    that C0111 was suppressed because this set of suppressions was from
> > >    mainline DTS and that has a lot of functions without
> > >    documentation.
> > 
> > Is this something we can fix as we go, migrating to DPDK main tree?
> > 
> > >  F0401 is disabled due to dependencies on TRex vendored
> > >    python libraries, since those will not be possible to import inside of
> > >    the container.
> > 
> > That seems fair enough to keep disabled.
> > 
> > > I don't remember why E731 is set, but it may be due to
> > >    the rte flow rule generator I wrote for mainline DTS, which makes use
> > >    of lambdas extensively to enable lazy evaluation, so that DTS doesn't
> > >    need to keep ~2 billion rules in memory.
> > 
> > That sounds reasonable too.
> > 
> > Can we perhaps target getting from 6 disabled warnings to 2 or 3 at most?
> > 
> > Other opinions?
> > 
> 
> We don't actually need the ignorelist at this point - I've removed the ignorelist, fixed a few issues and the tools don't report any problems. I'd like to start here and disable checks (either globally or in code where it's needed) when we actually need them (in a future patch).
> 
Great idea. Thanks.
  

<snip>

> 
> On Mon, Sep 12, 2022 at 10:06:03AM -0400, Owen Hilyard wrote:
> >    > E203 - whitespace before ‘,’, ‘;’, or ‘:’
> >    > E266 - too many leading ‘#’ for block comment
> >    > E501 - line too long
> >    > E731 - do not assign a lambda expression, use a def
> >    > C0111 - Missing %s docstring
> >    > F0401 - Unable to import %s
> >    E203, E266 and E501 were disabled due to pylama fighting with the
> >    autoformatters, so I decided to let the autoformatters win.
> 
> That sounds strange. Is there no way to configure the auto-formatters to avoid
> these issues? E203 I think we definitely should look to re-enable in DTS.
> 
> > I think
> >    that C0111 was suppressed because this set of suppressions was from
> >    mainline DTS and that has a lot of functions without
> >    documentation.
> 
> Is this something we can fix as we go, migrating to DPDK main tree?
Is C0111 useful from auto-generated documentation perspective? If yes, should not be ignored?
Existing code can be fixed as we bring it into DPDK repo.

> 
> >  F0401 is disabled due to dependencies on TRex vendored
> >    python libraries, since those will not be possible to import inside of
> >    the container.
> 
> That seems fair enough to keep disabled.
> 
> > I don't remember why E731 is set, but it may be due to
> >    the rte flow rule generator I wrote for mainline DTS, which makes use
> >    of lambdas extensively to enable lazy evaluation, so that DTS doesn't
> >    need to keep ~2 billion rules in memory.
> 
> That sounds reasonable too.
> 
> Can we perhaps target getting from 6 disabled warnings to 2 or 3 at most?
> 
> Other opinions?
> 
> /Bruce
  

<snip>

> >
> > On Fri, Jul 29, 2022 at 10:55:42AM +0000, Juraj Linkeš wrote:
> > > .gitignore contains standard Python-related files.
> > >
> > > Apart from that, add configuration for Python tools used in DTS:
> > > Poetry, dependency and package manager Black, formatter Pylama,
> > > static analysis Isort, import sorting
> > >
> > > .editorconfig modifies the line length to 88, which is the default
> > > Black uses. It seems to be the best of all worlds. [0]
> > >
> > > [0]
> > > https://black.readthedocs.io/en/stable/the_black_code_style/current_
> > > st
> > > yle.html#line-length
> > >
> > > Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu>
> > > Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> >
> > Thanks for the work on this. Some review comments inline below.
> >
> > /Bruce
> >
> > > ---
> > >  dts/.editorconfig  |   7 +
> > >  dts/.gitignore     |  14 ++
> > >  dts/README.md      |  15 ++
> > >  dts/poetry.lock    | 474
> > +++++++++++++++++++++++++++++++++++++++++++++
> > >  dts/pylama.ini     |   8 +
> > >  dts/pyproject.toml |  43 ++++
> > >  6 files changed, 561 insertions(+)
> > >  create mode 100644 dts/.editorconfig  create mode 100644
> > > dts/.gitignore  create mode 100644 dts/README.md  create mode 100644
> > > dts/poetry.lock  create mode 100644 dts/pylama.ini  create mode
> > > 100644 dts/pyproject.toml
> > >
> > > diff --git a/dts/.editorconfig b/dts/.editorconfig new file mode
> > > 100644 index 0000000000..657f959030
> > > --- /dev/null
> > > +++ b/dts/.editorconfig
> > > @@ -0,0 +1,7 @@
> > > +# SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2022
> > > +PANTHEON.tech s.r.o.
> > > +# See https://editorconfig.org/ for syntax reference.
> > > +#
> > > +
> > > +[*.py]
> > > +max_line_length = 88
> >
> > It seems strange to have two different editorconfig settings in DPDK.
> > Is there a reason that:
> > a) we can't use 79, the current DPDK default and recommended length by
> >    pycodestyle? Or alternatively:
> > b) change all of DPDK to use the 88 setting?
> >
> > Also, 88 seems an unusual number. How was it chosen/arrived at?
> >
> 
> The commit message contains a link to Black's documentation where they
> explain it:
> https://black.readthedocs.io/en/stable/the_black_code_style/current_style.ht
> ml#line-length
> 
> Let me know what you think about it. I think it's reasonable. I'll move the config
> to the top level .editorconfig file.
> 
> > > diff --git a/dts/.gitignore b/dts/.gitignore new file mode 100644
> > > index 0000000000..9c49935b6f
> > > --- /dev/null
> > > +++ b/dts/.gitignore
> > > @@ -0,0 +1,14 @@
> > > +# SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2022
> > > +PANTHEON.tech s.r.o.
> > > +#
> > > +
> > > +# Byte-compiled / optimized / DLL files __pycache__/ *.py[cod]
> > > +*$py.class
> > > +
> > > +# IDE files
> > > +.idea
> > > +
> > > +# DTS results
> > > +output
> >
> > I think this should be ok to merge into the main DPDK .gitignore file.
> >
> 
> Ok, I'll move it there.
> A sidenote - should I add Pantheon to the licence header?
I typically add the copyright if the change is significant (in anyway).

<snip>
  

<snip>

> > > > >
> > > > > diff --git a/dts/.editorconfig b/dts/.editorconfig new file mode
> > > > > 100644 index 0000000000..657f959030
> > > > > --- /dev/null
> > > > > +++ b/dts/.editorconfig
> > > > > @@ -0,0 +1,7 @@
> > > > > +# SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2022
> > > > > +PANTHEON.tech s.r.o.
> > > > > +# See https://editorconfig.org/ for syntax reference.
> > > > > +#
> > > > > +
> > > > > +[*.py]
> > > > > +max_line_length = 88
> > > >
> > > > It seems strange to have two different editorconfig settings in
> > > > DPDK. Is there a reason that:
> > > > a) we can't use 79, the current DPDK default and recommended length by
> > > >    pycodestyle? Or alternatively:
> > > > b) change all of DPDK to use the 88 setting?
> > > >
> > > > Also, 88 seems an unusual number. How was it chosen/arrived at?
> > > >
> > >
> > > The commit message contains a link to Black's documentation where
> > > they
> > explain it:
> > > https://black.readthedocs.io/en/stable/the_black_code_style/current_
> > > st
> > > yle.html#line-length
> > >
> > > Let me know what you think about it. I think it's reasonable. I'll
> > > move the
> > config to the top level .editorconfig file.
> > >
> >
> > I have no objection to moving this to the top level, but others may
> > like to keep our python style as standard. Realistically I see three choices here:
> >
> > 1. Force DTS to conform to existing DPDK python style of 79 characters
> > 2. Allow DTS to use 88 chars but the rest of DPDK to keep with 79
> > chars 3. Allow all of DPDK to use 88 chars.
> >
> > Of the 3, I like relaxing the 79/80 char limit so #3 seems best to me
> > as you suggest. However, I'd wait a few days for a desenting opinion
> > before I'd do a new patchset revision. :-)
+1 for option #3, it seems reasonable

> >
> 
> Ok, I'll wait.
> 
> > /Bruce
  

13/09/2022 21:19, Honnappa Nagarahalli:
> > > > > > --- /dev/null
> > > > > > +++ b/dts/.editorconfig
> > > > > > @@ -0,0 +1,7 @@
> > > > > > +# SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2022
> > > > > > +PANTHEON.tech s.r.o.
> > > > > > +# See https://editorconfig.org/ for syntax reference.
> > > > > > +#
> > > > > > +
> > > > > > +[*.py]
> > > > > > +max_line_length = 88
> > > > >
> > > > > It seems strange to have two different editorconfig settings in
> > > > > DPDK. Is there a reason that:
> > > > > a) we can't use 79, the current DPDK default and recommended length by
> > > > >    pycodestyle? Or alternatively:
> > > > > b) change all of DPDK to use the 88 setting?
> > > > >
> > > > > Also, 88 seems an unusual number. How was it chosen/arrived at?
> > > > >
> > > >
> > > > The commit message contains a link to Black's documentation where
> > > > they
> > > explain it:
> > > > https://black.readthedocs.io/en/stable/the_black_code_style/current_
> > > > st
> > > > yle.html#line-length
> > > >
> > > > Let me know what you think about it. I think it's reasonable. I'll
> > > > move the
> > > config to the top level .editorconfig file.
> > > >
> > >
> > > I have no objection to moving this to the top level, but others may
> > > like to keep our python style as standard. Realistically I see three choices here:
> > >
> > > 1. Force DTS to conform to existing DPDK python style of 79 characters
> > > 2. Allow DTS to use 88 chars but the rest of DPDK to keep with 79
> > > chars 3. Allow all of DPDK to use 88 chars.
> > >
> > > Of the 3, I like relaxing the 79/80 char limit so #3 seems best to me
> > > as you suggest. However, I'd wait a few days for a desenting opinion
> > > before I'd do a new patchset revision. :-)
> +1 for option #3, it seems reasonable

In .editorconfig, we have this default:
	max_line_length = 100
It is also documented in our guide.

For Python we have this exception:
	[*.py]
	indent_style = space
	indent_size = 4
	max_line_length = 79

I'm OK to increase it to 88 as it does not exceed the default.
In any case, it should be documented in the style guide.
  

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Wednesday, September 14, 2022 11:37 AM
> To: Juraj Linkeš <juraj.linkes@pantheon.tech>; Bruce Richardson
> <bruce.richardson@intel.com>; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>
> Cc: david.marchand@redhat.com; ronan.randles@intel.com;
> ohilyard@iol.unh.edu; lijuan.tu@intel.com; dev@dpdk.org
> Subject: Re: [PATCH v4 1/9] dts: add project tools config
> 
> 13/09/2022 21:19, Honnappa Nagarahalli:
> > > > > > > --- /dev/null
> > > > > > > +++ b/dts/.editorconfig
> > > > > > > @@ -0,0 +1,7 @@
> > > > > > > +# SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2022
> > > > > > > +PANTHEON.tech s.r.o.
> > > > > > > +# See https://editorconfig.org/ for syntax reference.
> > > > > > > +#
> > > > > > > +
> > > > > > > +[*.py]
> > > > > > > +max_line_length = 88
> > > > > >
> > > > > > It seems strange to have two different editorconfig settings
> > > > > > in DPDK. Is there a reason that:
> > > > > > a) we can't use 79, the current DPDK default and recommended length
> by
> > > > > >    pycodestyle? Or alternatively:
> > > > > > b) change all of DPDK to use the 88 setting?
> > > > > >
> > > > > > Also, 88 seems an unusual number. How was it chosen/arrived at?
> > > > > >
> > > > >
> > > > > The commit message contains a link to Black's documentation
> > > > > where they
> > > > explain it:
> > > > > https://black.readthedocs.io/en/stable/the_black_code_style/curr
> > > > > ent_
> > > > > st
> > > > > yle.html#line-length
> > > > >
> > > > > Let me know what you think about it. I think it's reasonable.
> > > > > I'll move the
> > > > config to the top level .editorconfig file.
> > > > >
> > > >
> > > > I have no objection to moving this to the top level, but others
> > > > may like to keep our python style as standard. Realistically I see three
> choices here:
> > > >
> > > > 1. Force DTS to conform to existing DPDK python style of 79
> > > > characters 2. Allow DTS to use 88 chars but the rest of DPDK to
> > > > keep with 79 chars 3. Allow all of DPDK to use 88 chars.
> > > >
> > > > Of the 3, I like relaxing the 79/80 char limit so #3 seems best to
> > > > me as you suggest. However, I'd wait a few days for a desenting
> > > > opinion before I'd do a new patchset revision. :-)
> > +1 for option #3, it seems reasonable
> 
> In .editorconfig, we have this default:
> 	max_line_length = 100
> It is also documented in our guide.
> 
> For Python we have this exception:
> 	[*.py]
> 	indent_style = space
> 	indent_size = 4
> 	max_line_length = 79
> 
> I'm OK to increase it to 88 as it does not exceed the default.
> In any case, it should be documented in the style guide.
> 

Where is the best place to document it? I'm thinking of adding a DTS Coding Style into contributor's guidelines, right after DPDK Coding Style. Or do we want to have separate DTS docs?

> 
>
  

On Wed, Sep 14, 2022 at 12:55:21PM +0000, Juraj Linkeš wrote:
> 
> 
> > -----Original Message----- From: Thomas Monjalon <thomas@monjalon.net>
> > Sent: Wednesday, September 14, 2022 11:37 AM To: Juraj Linkeš
> > <juraj.linkes@pantheon.tech>; Bruce Richardson
> > <bruce.richardson@intel.com>; Honnappa Nagarahalli
> > <Honnappa.Nagarahalli@arm.com> Cc: david.marchand@redhat.com;
> > ronan.randles@intel.com; ohilyard@iol.unh.edu; lijuan.tu@intel.com;
> > dev@dpdk.org Subject: Re: [PATCH v4 1/9] dts: add project tools config
> > 
> > 13/09/2022 21:19, Honnappa Nagarahalli:
> > > > > > > > --- /dev/null +++ b/dts/.editorconfig @@ -0,0 +1,7 @@ +#
> > > > > > > > SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2022
> > > > > > > > +PANTHEON.tech s.r.o.  +# See https://editorconfig.org/ for
> > > > > > > > syntax reference.  +# + +[*.py] +max_line_length = 88
> > > > > > >
> > > > > > > It seems strange to have two different editorconfig settings
> > > > > > > in DPDK. Is there a reason that: a) we can't use 79, the
> > > > > > > current DPDK default and recommended length
> > by
> > > > > > >    pycodestyle? Or alternatively: b) change all of DPDK to
> > > > > > >    use the 88 setting?
> > > > > > >
> > > > > > > Also, 88 seems an unusual number. How was it chosen/arrived
> > > > > > > at?
> > > > > > >
> > > > > >
> > > > > > The commit message contains a link to Black's documentation
> > > > > > where they
> > > > > explain it:
> > > > > > https://black.readthedocs.io/en/stable/the_black_code_style/curr
> > > > > > ent_ st yle.html#line-length
> > > > > >
> > > > > > Let me know what you think about it. I think it's reasonable.
> > > > > > I'll move the
> > > > > config to the top level .editorconfig file.
> > > > > >
> > > > >
> > > > > I have no objection to moving this to the top level, but others
> > > > > may like to keep our python style as standard. Realistically I
> > > > > see three
> > choices here:
> > > > >
> > > > > 1. Force DTS to conform to existing DPDK python style of 79
> > > > > characters 2. Allow DTS to use 88 chars but the rest of DPDK to
> > > > > keep with 79 chars 3. Allow all of DPDK to use 88 chars.
> > > > >
> > > > > Of the 3, I like relaxing the 79/80 char limit so #3 seems best
> > > > > to me as you suggest. However, I'd wait a few days for a
> > > > > desenting opinion before I'd do a new patchset revision. :-)
> > > +1 for option #3, it seems reasonable
> > 
> > In .editorconfig, we have this default: max_line_length = 100 It is
> > also documented in our guide.
> > 
> > For Python we have this exception: [*.py] indent_style = space
> > indent_size = 4 max_line_length = 79
> > 
> > I'm OK to increase it to 88 as it does not exceed the default.  In any
> > case, it should be documented in the style guide.
> > 
> 
> Where is the best place to document it? I'm thinking of adding a DTS
> Coding Style into contributor's guidelines, right after DPDK Coding
> Style. Or do we want to have separate DTS docs?

+1 for having it in the existing contributors doc. We want people to submit
code + DTS tests at the same time, so having the contribution docs combined
will help with this. It also should help us to try and align the two coding
styles for python. In future, we should perhaps update DPDK python code to
align to DTS coding style rather than having two styles in the one repo.

/Bruce
  

14/09/2022 15:11, Bruce Richardson:
> On Wed, Sep 14, 2022 at 12:55:21PM +0000, Juraj Linkeš wrote:
> > From: Thomas Monjalon <thomas@monjalon.net>
> > > 13/09/2022 21:19, Honnappa Nagarahalli:
> > > > > > > > > --- /dev/null +++ b/dts/.editorconfig @@ -0,0 +1,7 @@ +#
> > > > > > > > > SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2022
> > > > > > > > > +PANTHEON.tech s.r.o.  +# See https://editorconfig.org/ for
> > > > > > > > > syntax reference.  +# + +[*.py] +max_line_length = 88
> > > > > > > >
> > > > > > > > It seems strange to have two different editorconfig settings
> > > > > > > > in DPDK. Is there a reason that: a) we can't use 79, the
> > > > > > > > current DPDK default and recommended length
> > > by
> > > > > > > >    pycodestyle? Or alternatively: b) change all of DPDK to
> > > > > > > >    use the 88 setting?
> > > > > > > >
> > > > > > > > Also, 88 seems an unusual number. How was it chosen/arrived
> > > > > > > > at?
> > > > > > > >
> > > > > > >
> > > > > > > The commit message contains a link to Black's documentation
> > > > > > > where they
> > > > > > explain it:
> > > > > > > https://black.readthedocs.io/en/stable/the_black_code_style/curr
> > > > > > > ent_ st yle.html#line-length
> > > > > > >
> > > > > > > Let me know what you think about it. I think it's reasonable.
> > > > > > > I'll move the
> > > > > > config to the top level .editorconfig file.
> > > > > > >
> > > > > >
> > > > > > I have no objection to moving this to the top level, but others
> > > > > > may like to keep our python style as standard. Realistically I
> > > > > > see three
> > > choices here:
> > > > > >
> > > > > > 1. Force DTS to conform to existing DPDK python style of 79
> > > > > > characters 2. Allow DTS to use 88 chars but the rest of DPDK to
> > > > > > keep with 79 chars 3. Allow all of DPDK to use 88 chars.
> > > > > >
> > > > > > Of the 3, I like relaxing the 79/80 char limit so #3 seems best
> > > > > > to me as you suggest. However, I'd wait a few days for a
> > > > > > desenting opinion before I'd do a new patchset revision. :-)
> > > > +1 for option #3, it seems reasonable
> > > 
> > > In .editorconfig, we have this default: max_line_length = 100 It is
> > > also documented in our guide.
> > > 
> > > For Python we have this exception: [*.py] indent_style = space
> > > indent_size = 4 max_line_length = 79
> > > 
> > > I'm OK to increase it to 88 as it does not exceed the default.  In any
> > > case, it should be documented in the style guide.
> > > 
> > 
> > Where is the best place to document it? I'm thinking of adding a DTS
> > Coding Style into contributor's guidelines, right after DPDK Coding
> > Style. Or do we want to have separate DTS docs?
> 
> +1 for having it in the existing contributors doc. We want people to submit
> code + DTS tests at the same time, so having the contribution docs combined
> will help with this. It also should help us to try and align the two coding
> styles for python. In future, we should perhaps update DPDK python code to
> align to DTS coding style rather than having two styles in the one repo.

+1 for targetting unified coding style
  

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Wednesday, September 14, 2022 4:29 PM
> To: Juraj Linkeš <juraj.linkes@pantheon.tech>; Bruce Richardson
> <bruce.richardson@intel.com>
> Cc: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>;
> david.marchand@redhat.com; ronan.randles@intel.com; ohilyard@iol.unh.edu;
> lijuan.tu@intel.com; dev@dpdk.org
> Subject: Re: [PATCH v4 1/9] dts: add project tools config
> 
> 14/09/2022 15:11, Bruce Richardson:
> > On Wed, Sep 14, 2022 at 12:55:21PM +0000, Juraj Linkeš wrote:
> > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > 13/09/2022 21:19, Honnappa Nagarahalli:
> > > > > > > > > > --- /dev/null +++ b/dts/.editorconfig @@ -0,0 +1,7 @@
> > > > > > > > > > +#
> > > > > > > > > > SPDX-License-Identifier: BSD-3-Clause # Copyright(c)
> > > > > > > > > > 2022
> > > > > > > > > > +PANTHEON.tech s.r.o.  +# See
> > > > > > > > > > +https://editorconfig.org/ for
> > > > > > > > > > syntax reference.  +# + +[*.py] +max_line_length = 88
> > > > > > > > >
> > > > > > > > > It seems strange to have two different editorconfig
> > > > > > > > > settings in DPDK. Is there a reason that: a) we can't
> > > > > > > > > use 79, the current DPDK default and recommended length
> > > > by
> > > > > > > > >    pycodestyle? Or alternatively: b) change all of DPDK to
> > > > > > > > >    use the 88 setting?
> > > > > > > > >
> > > > > > > > > Also, 88 seems an unusual number. How was it
> > > > > > > > > chosen/arrived at?
> > > > > > > > >
> > > > > > > >
> > > > > > > > The commit message contains a link to Black's
> > > > > > > > documentation where they
> > > > > > > explain it:
> > > > > > > > https://black.readthedocs.io/en/stable/the_black_code_styl
> > > > > > > > e/curr ent_ st yle.html#line-length
> > > > > > > >
> > > > > > > > Let me know what you think about it. I think it's reasonable.
> > > > > > > > I'll move the
> > > > > > > config to the top level .editorconfig file.
> > > > > > > >
> > > > > > >
> > > > > > > I have no objection to moving this to the top level, but
> > > > > > > others may like to keep our python style as standard.
> > > > > > > Realistically I see three
> > > > choices here:
> > > > > > >
> > > > > > > 1. Force DTS to conform to existing DPDK python style of 79
> > > > > > > characters 2. Allow DTS to use 88 chars but the rest of DPDK
> > > > > > > to keep with 79 chars 3. Allow all of DPDK to use 88 chars.
> > > > > > >
> > > > > > > Of the 3, I like relaxing the 79/80 char limit so #3 seems
> > > > > > > best to me as you suggest. However, I'd wait a few days for
> > > > > > > a desenting opinion before I'd do a new patchset revision.
> > > > > > > :-)
> > > > > +1 for option #3, it seems reasonable
> > > >
> > > > In .editorconfig, we have this default: max_line_length = 100 It
> > > > is also documented in our guide.
> > > >
> > > > For Python we have this exception: [*.py] indent_style = space
> > > > indent_size = 4 max_line_length = 79
> > > >
> > > > I'm OK to increase it to 88 as it does not exceed the default.  In
> > > > any case, it should be documented in the style guide.
> > > >
> > >
> > > Where is the best place to document it? I'm thinking of adding a DTS
> > > Coding Style into contributor's guidelines, right after DPDK Coding
> > > Style. Or do we want to have separate DTS docs?
> >
> > +1 for having it in the existing contributors doc. We want people to
> > +submit
> > code + DTS tests at the same time, so having the contribution docs
> > combined will help with this. It also should help us to try and align
> > the two coding styles for python. In future, we should perhaps update
> > DPDK python code to align to DTS coding style rather than having two styles in
> the one repo.
> 
> +1 for targetting unified coding style
> 

Should I now just add the note about max line length and move the rest of DTS Python coding style when we unify the code? We have more we wanted to add [0], but that is really of the scope of this change.

[0] https://docs.google.com/document/d/1G7_AEA-4MAd88bxjfP-IOcIy_6mnrMz3HCsUI8e-NN4/edit?pli=1#heading=h.fkq21x8krlp1