On Fri, Sep 09, 2022 at 01:38:33PM +0000, Juraj Linkeš wrote:
> 
> 
> > -----Original Message-----
> > From: Bruce Richardson <bruce.richard...@intel.com>
> > Sent: Wednesday, September 7, 2022 6:17 PM
> > To: Juraj Linkeš <juraj.lin...@pantheon.tech>
> > Cc: tho...@monjalon.net; david.march...@redhat.com;
> > ronan.rand...@intel.com; honnappa.nagaraha...@arm.com;
> > ohily...@iol.unh.edu; lijuan...@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 <ohily...@iol.unh.edu>
> > > Signed-off-by: Juraj Linkeš <juraj.lin...@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

Reply via email to