All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed

From: Ed Bartosh <ed.bartosh@linux.intel.com>
To: Michael Wood <michael.g.wood@intel.com>
Cc: toaster@yoctoproject.org
Subject: Re: [review-request] ed/toaster/misc
Date: Tue, 14 Jul 2015 16:06:36 +0300	[thread overview]
Message-ID: <20150714130636.GA25220@linux.intel.com> (raw)
In-Reply-To: <55A3EBB9.5060109@intel.com>

Hi Michael,

Thanks for the review.

...

> While you're in localhostbecontroller
> 
> We could also get rid of
> 
>     def _shellcmd(self, command, cwd = None):
>         if cwd is None:
>             cwd = self.be.sourcedir
> 
>         #logger.debug("lbc_shellcmmd: (%s) %s" % (cwd, command))
>         p = subprocess.Popen(command, cwd = cwd, shell=True,
> stdout=subprocess.PIPE, stderr=subprocess.PIPE)
>         (out,err) = p.communicate()
>         p.wait()
>         if p.returncode:
>             if len(err) == 0:
>                 err = "command: %s \n%s" % (command, out)
>             else:
>                 err = "command: %s \n%s" % (command, err)
>             #logger.warn("localhostbecontroller: shellcmd error %s" % err)
>             raise ShellCmdException(err)
>         else:
>             #logger.debug("localhostbecontroller: shellcmd success")
>             return out
> 
> 
> I believe with:
> 
> subprocess.check_output
> 
Yep, I've noticed this too. I think the reason for not using
check_output was that it doesn't have cwd parameter.

I'm planning to generalize build controllers, so I'll make much more
changes to this code anyway. Hopefully it will be for good :)
Just discussed this with Alex and he seems to be ok with this.

--
Regards,
Ed

next prev parent reply	other threads:[~2015-07-14 13:08 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-10 14:48 [review-request] ed/toaster/misc Ed Bartosh
2015-07-13 16:47 ` Michael Wood
2015-07-14 11:18   ` Damian, Alexandru
2015-07-14 13:37     ` Ed Bartosh
2015-07-19 19:35       ` Ed Bartosh
2015-07-14 13:06   ` Ed Bartosh [this message]
2015-07-14 13:37     ` Damian, Alexandru
2015-07-14 13:52       ` Ed Bartosh
2015-07-24 16:22         ` Ed Bartosh
2015-07-27 12:26           ` Damian, Alexandru
2015-07-28 12:35             ` Ed Bartosh
2015-07-28 14:00               ` Damian, Alexandru

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150714130636.GA25220@linux.intel.com \
    --to=ed.bartosh@linux.intel.com \
    --cc=michael.g.wood@intel.com \
    --cc=toaster@yoctoproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.