Re: [PATCH 1/2] distro_check.py: Add a new function to reduce the repeat code

[Joshua Lock <josh@linux.intel.com>]

On Thu, 2011-01-13 at 15:04 +0800, Mei Lei wrote:
> From: Mei Lei <lei.mei@intel.com>
> 
> A lot of repeat code in distrodata.bbclass when creating log file, we
> can define a function called create_log_file to do it.

I like the intent of the patch, some comments on coding style below.

> 
> Signed-off-by: Mei Lei <lei.mei@intel.com>
> ---
>  meta/lib/oe/distro_check.py |   15 +++++++++++++++
>  1 files changed, 15 insertions(+), 0 deletions(-)
> 
> diff --git a/meta/lib/oe/distro_check.py b/meta/lib/oe/distro_check.py
> index c85d4fb..134e92a 100644
> --- a/meta/lib/oe/distro_check.py
> +++ b/meta/lib/oe/distro_check.py
> @@ -341,6 +341,21 @@ def compare_in_distro_packages_list(distro_check_dir, d):
>      bb.note("Matching: %s" % matching_distros)
>      return matching_distros
>  
> +def create_log_file(d, logname):
> +    #logpath = bb.data.getVar('LOG_DIR', e.data, 1)

No need for this commented line.

> +    logpath = bb.data.getVar('LOG_DIR', d, 1)

We are trying to move towards using Boolean values where appropriate to
aid readability, this line should be:

logpath = bb.data.getVar('LOG_DIR', d, True)

> +    bb.utils.mkdirhier(logpath)
> +    logfile = logpath +"/" + logname.split(".")[0] + ".%s.csv" % bb.data.getVar('DATETIME', d, 1)

Can you use the Python standard library functions for handling filenames
here? And a boolean True in the getVar() call.

Something like the following feels more Pythonic:

logfn, logext = os.path.splitext(logname)
logfile = os.path.join(logpath, "%s.%s.csv" % (logfn,
bb.data.getVar('DATETIME', d, True))

> +    if not os.path.exists(logfile):
> +            slogfile = os.path.join(logpath, logname)
> +            if os.path.exists(slogfile):
> +                    os.remove(slogfile)
> +            os.system("touch %s" % logfile)
> +            os.symlink(logfile, slogfile)
> +            bb.data.setVar('LOG_FILE', logfile, d)
> +    return logfile
> +
> +
>  def save_distro_check_result(result, datetime, d):
>      pn = bb.data.getVar('PN', d, True)
>      logdir = bb.data.getVar('LOG_DIR', d, True)

-- 
Joshua Lock
        Yocto Build System Monkey
        Intel Open Source Technology Centre