<div dir="ltr"><div dir="ltr">> 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. </div> <div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Sep 9, 2022 at 10:13 AM Juraj Linkeš <juraj.linkes@pantheon.tech> wrote: </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> > -----Original Message----- > From: Bruce Richardson <<a href="mailto:bruce.richardson@intel.com" target="_blank">bruce.richardson@intel.com</a>> > Sent: Friday, September 9, 2022 3:53 PM > To: Juraj Linkeš <juraj.linkes@pantheon.tech> > Cc: <a href="mailto:thomas@monjalon.net" target="_blank">thomas@monjalon.net</a>; <a href="mailto:david.marchand@redhat.com" target="_blank">david.marchand@redhat.com</a>; > <a href="mailto:ronan.randles@intel.com" target="_blank">ronan.randles@intel.com</a>; <a href="mailto:Honnappa.Nagarahalli@arm.com" target="_blank">Honnappa.Nagarahalli@arm.com</a>; > <a href="mailto:ohilyard@iol.unh.edu" target="_blank">ohilyard@iol.unh.edu</a>; <a href="mailto:lijuan.tu@intel.com" target="_blank">lijuan.tu@intel.com</a>; <a href="mailto:dev@dpdk.org" target="_blank">dev@dpdk.org</a> > 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 <<a href="mailto:bruce.richardson@intel.com" target="_blank">bruce.richardson@intel.com</a>> > > > Sent: Wednesday, September 7, 2022 6:17 PM > > > To: Juraj Linkeš <juraj.linkes@pantheon.tech> > > > Cc: <a href="mailto:thomas@monjalon.net" target="_blank">thomas@monjalon.net</a>; <a href="mailto:david.marchand@redhat.com" target="_blank">david.marchand@redhat.com</a>; > > > <a href="mailto:ronan.randles@intel.com" target="_blank">ronan.randles@intel.com</a>; <a href="mailto:Honnappa.Nagarahalli@arm.com" target="_blank">Honnappa.Nagarahalli@arm.com</a>; > > > <a href="mailto:ohilyard@iol.unh.edu" target="_blank">ohilyard@iol.unh.edu</a>; <a href="mailto:lijuan.tu@intel.com" target="_blank">lijuan.tu@intel.com</a>; <a href="mailto:dev@dpdk.org" target="_blank">dev@dpdk.org</a> > > > 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] > > > > <a href="https://black.readthedocs.io/en/stable/the_black_code_style/curren" rel="noreferrer" target="_blank">https://black.readthedocs.io/en/stable/the_black_code_style/curren</a> > > > > t_st > > > > yle.html#line-length > > > > > > > > Signed-off-by: Owen Hilyard <<a href="mailto:ohilyard@iol.unh.edu" target="_blank">ohilyard@iol.unh.edu</a>> > > > > 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 <a href="https://editorconfig.org/" rel="noreferrer" target="_blank">https://editorconfig.org/</a> 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: > > <a href="https://black.readthedocs.io/en/stable/the_black_code_style/current_st" rel="noreferrer" target="_blank">https://black.readthedocs.io/en/stable/the_black_code_style/current_st</a> > > 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 </blockquote></div></div>