From: Owen Hilyard <ohilyard@iol.unh.edu>
To: "Juraj Linkeš" <juraj.linkes@pantheon.tech>
Cc: Bruce Richardson <bruce.richardson@intel.com>,
"thomas@monjalon.net" <thomas@monjalon.net>,
"david.marchand@redhat.com" <david.marchand@redhat.com>,
"ronan.randles@intel.com" <ronan.randles@intel.com>,
"Honnappa.Nagarahalli@arm.com" <Honnappa.Nagarahalli@arm.com>,
"lijuan.tu@intel.com" <lijuan.tu@intel.com>,
"dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [PATCH v4 1/9] dts: add project tools config
Date: Mon, 12 Sep 2022 10:06:03 -0400 [thread overview]
Message-ID: <CAHx6DYCw=CHy+ReRG3zL-WJg28s1-BEuJKsz7Potf8N5GDM-8Q@mail.gmail.com> (raw)
In-Reply-To: <bc7b4cc8b0ac476793de4d65619e7e27@pantheon.tech>
[-- Attachment #1: Type: text/plain, Size: 5359 bytes --]
> 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
>
>
[-- Attachment #2: Type: text/html, Size: 8555 bytes --]
next prev parent reply other threads:[~2022-09-12 14:06 UTC|newest]
Thread overview: 105+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-06-22 12:14 [PATCH v1 0/8] dts: ssh connection to a node Juraj Linkeš
2022-06-22 12:14 ` [PATCH v1 1/8] dts: add ssh pexpect library Juraj Linkeš
2022-06-22 12:14 ` [PATCH v1 2/8] dts: add locks for parallel node connections Juraj Linkeš
2022-06-22 12:14 ` [PATCH v1 3/8] dts: add ssh connection extension Juraj Linkeš
2022-06-22 12:14 ` [PATCH v1 4/8] dts: add basic logging facility Juraj Linkeš
2022-06-22 12:14 ` [PATCH v1 5/8] dts: add Node base class Juraj Linkeš
2022-06-22 12:14 ` [PATCH v1 6/8] dts: add config parser module Juraj Linkeš
2022-06-22 12:14 ` [PATCH v1 7/8] dts: add dts runtime workflow module Juraj Linkeš
2022-06-22 12:14 ` [PATCH v1 8/8] dts: add main script for running dts Juraj Linkeš
2022-07-11 14:51 ` [PATCH v2 0/8] ssh connection to a node Juraj Linkeš
2022-07-11 14:51 ` [PATCH v2 1/8] dts: add basic logging facility Juraj Linkeš
2022-07-11 14:51 ` [PATCH v2 2/8] dts: add ssh pexpect library Juraj Linkeš
2022-07-11 14:51 ` [PATCH v2 3/8] dts: add locks for parallel node connections Juraj Linkeš
2022-07-11 14:51 ` [PATCH v2 4/8] dts: add ssh connection extension Juraj Linkeš
2022-07-11 14:51 ` [PATCH v2 5/8] dts: add config parser module Juraj Linkeš
2022-07-11 14:51 ` [PATCH v2 6/8] dts: add Node base class Juraj Linkeš
2022-07-11 14:51 ` [PATCH v2 7/8] dts: add dts workflow module Juraj Linkeš
2022-07-11 14:51 ` [PATCH v2 8/8] dts: add dts executable script Juraj Linkeš
2022-07-28 10:00 ` [PATCH v3 0/9] dts: ssh connection to a node Juraj Linkeš
2022-07-28 10:00 ` [PATCH v3 1/9] dts: add project tools config Juraj Linkeš
2022-07-28 10:00 ` [PATCH v3 2/9] dts: add developer tools Juraj Linkeš
2022-07-28 10:00 ` [PATCH v3 3/9] dts: add basic logging facility Juraj Linkeš
2022-07-28 10:00 ` [PATCH v3 4/9] dts: add ssh pexpect library Juraj Linkeš
2022-07-28 10:00 ` [PATCH v3 5/9] dts: add ssh connection extension Juraj Linkeš
2022-07-28 10:00 ` [PATCH v3 6/9] dts: add config parser module Juraj Linkeš
2022-07-28 10:00 ` [PATCH v3 7/9] dts: add Node base class Juraj Linkeš
2022-07-28 10:00 ` [PATCH v3 8/9] dts: add dts workflow module Juraj Linkeš
2022-07-28 10:00 ` [PATCH v3 9/9] dts: add dts executable script Juraj Linkeš
2022-07-29 10:55 ` [PATCH v4 0/9] dts: ssh connection to a node Juraj Linkeš
2022-07-29 10:55 ` [PATCH v4 1/9] dts: add project tools config Juraj Linkeš
2022-08-10 6:30 ` Tu, Lijuan
2022-09-07 16:16 ` Bruce Richardson
2022-09-09 13:38 ` Juraj Linkeš
2022-09-09 13:52 ` Bruce Richardson
2022-09-09 14:13 ` Juraj Linkeš
2022-09-12 14:06 ` Owen Hilyard [this message]
2022-09-12 15:15 ` Bruce Richardson
2022-09-13 12:08 ` Juraj Linkeš
2022-09-13 14:18 ` Bruce Richardson
2022-09-13 19:03 ` Honnappa Nagarahalli
2022-09-13 19:19 ` Honnappa Nagarahalli
2022-09-14 9:37 ` Thomas Monjalon
2022-09-14 12:55 ` Juraj Linkeš
2022-09-14 13:11 ` Bruce Richardson
2022-09-14 14:28 ` Thomas Monjalon
2022-09-21 10:49 ` Juraj Linkeš
2022-09-13 19:11 ` Honnappa Nagarahalli
2022-07-29 10:55 ` [PATCH v4 2/9] dts: add developer tools Juraj Linkeš
2022-08-10 6:30 ` Tu, Lijuan
2022-09-07 16:37 ` Bruce Richardson
2022-09-13 12:38 ` Juraj Linkeš
2022-09-13 20:38 ` Honnappa Nagarahalli
2022-09-14 7:37 ` Bruce Richardson
2022-09-14 12:45 ` Juraj Linkeš
2022-09-14 13:13 ` Bruce Richardson
2022-09-14 14:26 ` Thomas Monjalon
2022-09-14 19:08 ` Honnappa Nagarahalli
2022-09-20 12:14 ` Juraj Linkeš
2022-09-20 12:22 ` Tu, Lijuan
2022-07-29 10:55 ` [PATCH v4 3/9] dts: add basic logging facility Juraj Linkeš
2022-08-10 6:31 ` Tu, Lijuan
2022-09-08 8:31 ` Bruce Richardson
2022-09-13 12:52 ` Juraj Linkeš
2022-09-13 23:31 ` Honnappa Nagarahalli
2022-09-14 12:51 ` Juraj Linkeš
2022-07-29 10:55 ` [PATCH v4 4/9] dts: add ssh pexpect library Juraj Linkeš
2022-08-10 6:31 ` Tu, Lijuan
2022-09-08 9:53 ` Bruce Richardson
2022-09-13 13:36 ` Juraj Linkeš
2022-09-13 14:23 ` Bruce Richardson
2022-09-13 14:59 ` Stanislaw Kardach
2022-09-13 17:23 ` Owen Hilyard
2022-09-14 0:03 ` Honnappa Nagarahalli
2022-09-14 7:42 ` Bruce Richardson
2022-09-14 7:58 ` Stanislaw Kardach
2022-09-14 19:57 ` Honnappa Nagarahalli
2022-09-19 14:21 ` Owen Hilyard
2022-09-20 17:54 ` Honnappa Nagarahalli
2022-09-21 1:01 ` Tu, Lijuan
2022-09-21 5:37 ` Jerin Jacob
2022-09-22 9:03 ` Juraj Linkeš
2022-09-14 9:42 ` Stanislaw Kardach
2022-09-22 9:41 ` Juraj Linkeš
2022-09-22 14:32 ` Stanislaw Kardach
2022-09-23 7:22 ` Juraj Linkeš
2022-09-23 8:15 ` Bruce Richardson
2022-09-23 10:18 ` Stanislaw Kardach
2022-07-29 10:55 ` [PATCH v4 5/9] dts: add ssh connection extension Juraj Linkeš
2022-08-10 6:32 ` Tu, Lijuan
2022-09-13 17:04 ` Bruce Richardson
2022-09-13 17:32 ` Owen Hilyard
2022-09-14 7:46 ` Bruce Richardson
2022-09-14 12:02 ` Owen Hilyard
2022-09-14 13:15 ` Bruce Richardson
2022-07-29 10:55 ` [PATCH v4 6/9] dts: add config parser module Juraj Linkeš
2022-08-10 6:33 ` Tu, Lijuan
2022-09-13 17:19 ` Bruce Richardson
2022-09-13 17:47 ` Owen Hilyard
2022-09-14 7:48 ` Bruce Richardson
2022-07-29 10:55 ` [PATCH v4 7/9] dts: add Node base class Juraj Linkeš
2022-08-10 6:33 ` Tu, Lijuan
2022-07-29 10:55 ` [PATCH v4 8/9] dts: add dts workflow module Juraj Linkeš
2022-08-10 6:34 ` Tu, Lijuan
2022-07-29 10:55 ` [PATCH v4 9/9] dts: add dts executable script Juraj Linkeš
2022-08-10 6:35 ` Tu, Lijuan
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='CAHx6DYCw=CHy+ReRG3zL-WJg28s1-BEuJKsz7Potf8N5GDM-8Q@mail.gmail.com' \
--to=ohilyard@iol.unh.edu \
--cc=Honnappa.Nagarahalli@arm.com \
--cc=bruce.richardson@intel.com \
--cc=david.marchand@redhat.com \
--cc=dev@dpdk.org \
--cc=juraj.linkes@pantheon.tech \
--cc=lijuan.tu@intel.com \
--cc=ronan.randles@intel.com \
--cc=thomas@monjalon.net \
/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).