From: "Juraj Linkeš" <juraj.linkes@pantheon.tech>
To: Bruce Richardson <bruce.richardson@intel.com>
Cc: "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>,
"ohilyard@iol.unh.edu" <ohilyard@iol.unh.edu>,
"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: Fri, 9 Sep 2022 13:38:33 +0000 [thread overview]
Message-ID: <3680ad0ceb5944e6b5050d5f7b9c9599@pantheon.tech> (raw)
In-Reply-To: <YxjD61ZemoI+zEnE@bricha3-MOBL.ger.corp.intel.com>
> -----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>
>
next prev parent reply other threads:[~2022-09-09 13:38 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š [this message]
2022-09-09 13:52 ` Bruce Richardson
2022-09-09 14:13 ` Juraj Linkeš
2022-09-12 14:06 ` Owen Hilyard
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=3680ad0ceb5944e6b5050d5f7b9c9599@pantheon.tech \
--to=juraj.linkes@pantheon.tech \
--cc=Honnappa.Nagarahalli@arm.com \
--cc=bruce.richardson@intel.com \
--cc=david.marchand@redhat.com \
--cc=dev@dpdk.org \
--cc=lijuan.tu@intel.com \
--cc=ohilyard@iol.unh.edu \
--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).