[U-Boot] [PATCH v5 09/15] tools: add genboardscfg.py

[Simon Glass]

Hi Masahiro,

On 27 July 2014 10:05, Masahiro Yamada <yamada.m at jp.panasonic.com> wrote:
> Hi Simon,
>
>
>
> On Sat, 26 Jul 2014 01:17:02 +0100
> Simon Glass <sjg at chromium.org> wrote:
>
>> > +
>> > +import sys
>> > +import os
>> > +import errno
>> > +import shutil
>> > +import time
>> > +import subprocess
>> > +import fnmatch
>> > +import glob
>> > +import re
>> > +import optparse
>>
>> You can sort the imports.
>
>
> Sorted alphabetially in v6.
>
> Pardon my ignorance, but is there any coding style guide
> about sorting imports?

Not really. I have been following PEP8 (which I don't think mentions
this), but in general in U-Boot we try to sort include files.

>
>
>
>> > +
>> > +### helper functions ###
>> > +def get_terminal_columns():
>> > +    '''
>> > +    Get the width of the terminal
>> > +    Return 0 if the stdout is not associated with tty.
>> > +    '''
>> > +    try:
>> > +        return shutil.get_terminal_size().columns # Python 3.3~
>> > +    except:
>> > +        import fcntl
>> > +        import termios
>> > +        import struct
>> > +        arg = struct.pack('hhhh', 0, 0, 0, 0)
>> > +        try:
>> > +            ret = fcntl.ioctl(sys.stdout.fileno(), termios.TIOCGWINSZ, arg)
>>
>> Perhaps could use sys.version_info here and below?
>
>
> I think it will also work. But we should exactly know which version support
> the new method.
>
> Isn't it easier to use "try: ... except: ..." statement ?

Well your comment suggests that you do know what version supports this
feature so it could be changed to use 'if'. But it's not that
different - I think what you have is OK.

>
>
>> Also I hesitate to suggest this, but there is a terminal.py library in
>> patman. Should we consider moving that into a common directory and
>> putting all terminal-related code there?
>
>
> I do not see the advantage to do that for now.
> The terminal.py library has only Color class and this tool does not need it.
> (We could display warning messages in red text, but it would not be mandatory.)

OK, something to keep an eye on. I was more thinking of someone else
wanting access to the terminal code you have written here, from
another tool.

Regards,
Simon