On Tue, Sep 13, 2022 at 01:36:42PM +0000, Juraj Linkeš wrote:
> 
> 
> > -----Original Message-----
> > From: Bruce Richardson <bruce.richard...@intel.com>
> > Sent: Thursday, September 8, 2022 11:53 AM
> > 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 4/9] dts: add ssh pexpect library
> > 
> > On Fri, Jul 29, 2022 at 10:55:45AM +0000, Juraj Linkeš wrote:

<snip>

> > > +            self.send_expect("stty -echo", "#")
> > > +            self.send_expect("stty columns 1000", "#")
> > > +        except Exception as e:
> > > +            print(RED(str(e)))
> > > +            if getattr(self, "port", None):
> > > +                suggestion = (
> > > +                    "\nSuggession: Check if the firewall on [ %s ] " % 
> > > self.ip
> > > +                    + "is stopped\n"
> > > +                )
> > 
> > I'd suggest using f-strings here to avoid splitting error messages across 
> > lines.
> > They can also be used for strings above too to increase readability.
> > 
> > We should probably look to standardize all strings used in DTS to a single 
> > format
> > - either f-strings or the style given here, rather than using a mix.
> > 
> 
> This is one of the many things we left from the original code to facilitate 
> discussion - would this be a requirement or can we skip it (possibly changing 
> it later)? I prefer f-strings everywhere and I'll change it where I can, at 
> least in this patch.
> Maybe we could do this one patch the best we can to showcase what the code 
> should ideally look like and possibly loosen requirements in subsequent 
> patches? This will leave technical debt so it doesn't sound good.
> 

Yes, I can understand that a huge amount of tech-debt has built up in the
code, and it's probably a fairly huge undertaking to remove it all.
On the other hand, this move to the main repo seems the best opportunity we
are likely to get to try and clean this up and standardise it. Therefore,
I'd really like to see us use f-strings everywhere. Is there a
style-checker that can be used automatically to flag older-style strings?

Reply via email to