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

["Juraj Linkeš" <juraj.linkes@pantheon.tech>]

> -----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