On Thursday, December 5, 2019, Cleber Rosa <cr...@redhat.com> wrote: > On Mon, Dec 02, 2019 at 05:49:58PM -0300, Eduardo Habkost wrote: > > On Sun, Dec 01, 2019 at 12:46:12AM +0100, Aleksandar Markovic wrote: > > > On Monday, November 25, 2019, Filip Bozuta <filip.boz...@rt-rk.com> > wrote: > > > > > > > The script checkpatch.pl located in scripts folder was > > > > used to detect all errors and warrnings in files: > > > > hw/mips/mips_malta.c > > > > hw/mips/gt64xxx_pci.c > > > > tests/acceptance/linux_ssh_mips_malta.py > > > > > > > > All these mips malta machine files were edited and > > > > all the errors and warrings generated by the checkpatch.pl > > > > script were corrected and then the script was > > > > ran again to make sure there are no more errors and warnings. > > > > > > > > Signed-off-by: Filip Bozuta <filip.boz...@rt-rk.com> > > > > --- > > > > hw/mips/mips_malta.c | 139 > > > > ++++++++++++++++--------------- > > > > tests/acceptance/linux_ssh_mips_malta.py | 6 +- > > > > 2 files changed, 75 insertions(+), 70 deletions(-) > > > > > > > > > > > Very good cleanup! > > > > > > Reviewed-by: Aleksandar Markovic <amarko...@wavecomp.com> > > > > > > I think this snippet is good, but I am just in case cc-ing Cleber and > > > Eduardo to check if it is in accordance with any applicable guidelines > of > > > our test framework: > > > > Thanks for CCing us. > > > > > > > > > > > > diff --git a/tests/acceptance/linux_ssh_mips_malta.py > > > > b/tests/acceptance/linux_ssh_mips_malta.py > > > > index fc13f9e..44e1118 100644 > > > > --- a/tests/acceptance/linux_ssh_mips_malta.py > > > > +++ b/tests/acceptance/linux_ssh_mips_malta.py > > > > @@ -99,10 +99,12 @@ class LinuxSSH(Test): > > > > def ssh_command(self, command, is_root=True): > > > > self.ssh_logger.info(command) > > > > result = self.ssh_session.cmd(command) > > > > - stdout_lines = [line.rstrip() for line in > > > > result.stdout_text.splitlines()] > > > > + stdout_lines = [line.rstrip() for line > > > > + in result.stdout_text.splitlines()] > > > > for line in stdout_lines: > > > > self.ssh_logger.info(line) > > > > - stderr_lines = [line.rstrip() for line in > > > > result.stderr_text.splitlines()] > > > > + stderr_lines = [line.rstrip() for line > > > > + in result.stderr_text.splitlines()] > > > > If you really want to split those lines, please indent them > > properly. e.g.: > > > > stdout_lines = [line.rstrip() for line > > in result.stdout_text.splitlines()] > > > > See PEP 8 [1] for additional examples. > > > > Personally, I would just keep the existing > > linux_ssh_mips_malta.py code as is. I don't think splitting > > those lines improves readability. > > > > [1] https://www.python.org/dev/peps/pep-0008/#indentation > > > > -- > > Eduardo > > Right. It only helps when running terminal or editor settings are > limited to ~80 cols. And even in those cases, sometimes such code > split across lines needs a lot of gymnastics to not degrade in > readability when compared to a longer line. > > We're not (yet?) enforcing PEP 8, so either as Eduardo suggested, or > with no changes LGTM. > > Eduardo, Cleber,
I truly appreciate your responses and clarifications. Best regards, Aleksandar > Cheers, > - Cleber. > >
