From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by yocto-www.yoctoproject.org (Postfix, from userid 118) id 26AB9E00B7B; Wed, 11 Jun 2014 09:53:30 -0700 (PDT) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on yocto-www.yoctoproject.org X-Spam-Level: X-Spam-Status: No, score=0.0 required=5.0 tests=HTML_MESSAGE autolearn=ham version=3.3.1 X-Spam-HAM-Report: * 0.0 HTML_MESSAGE BODY: HTML included in message Received: from www.dynamicdevices.co.uk (www.dynamicdevices.co.uk [89.200.136.37]) by yocto-www.yoctoproject.org (Postfix) with ESMTP id B6783E0070F for ; Wed, 11 Jun 2014 09:53:17 -0700 (PDT) Received: from localhost (localhost [127.0.0.1]) by www.dynamicdevices.co.uk (Postfix) with ESMTP id 206DD27E02B; Wed, 11 Jun 2014 16:53:16 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at lennoab2.miniserver.com Received: from www.dynamicdevices.co.uk ([127.0.0.1]) by localhost (www.dynamicdevices.co.uk [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id uqQ7mOPce909; Wed, 11 Jun 2014 16:53:11 +0000 (UTC) Received: from [127.0.0.1] (cpc32-live22-2-0-cust59.17-2.cable.virginm.net [82.36.253.60]) (using TLSv1 with cipher ECDHE-RSA-AES128-SHA (128/128 bits)) (No client certificate requested) by www.dynamicdevices.co.uk (Postfix) with ESMTPSA id CF1C627E02A; Wed, 11 Jun 2014 16:53:11 +0000 (UTC) Message-ID: <53988974.6090409@dynamicdevices.co.uk> Date: Wed, 11 Jun 2014 17:53:08 +0100 From: Alex J Lennon User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:24.0) Gecko/20100101 Thunderbird/24.5.0 MIME-Version: 1.0 To: Andrei Gherzan References: In-Reply-To: X-Enigmail-Version: 1.6 Cc: Yocto Project Subject: Re: [meta-raspberrypi][PATCH 0/1] pi-blaster: Add recipe X-BeenThere: yocto@yoctoproject.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: Discussion of all things Yocto Project List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 11 Jun 2014 16:53:30 -0000 Content-Type: multipart/alternative; boundary="------------070804090807080901090900" --------------070804090807080901090900 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit On 10/06/2014 20:50, Andrei Gherzan wrote: > Hi Alex, > > > On Tue, May 20, 2014 at 5:40 PM, Alex J Lennon > > > wrote: > > Please see following patch for details. > > The following changes since commit > f3a8693f08f99893453fd1fe282515b2f222c080: > > omxplayer: Update to remote's HEAD (2014-05-09 14:56:59 +0300) > > are available in the git repository at: > > git://github.com/DynamicDevices/meta-raspberrypi > ajl/pi-blaster > https://github.com/DynamicDevices/meta-raspberrypi/tree/master > > Alex J Lennon (1): > pi-blaster: Added recipe > > recipes-devtools/pi-blaster/files/initscript.patch | 71 > ++++++++++++++++++++++ > recipes-devtools/pi-blaster/pi-blaster.inc | 36 +++++++++++ > recipes-devtools/pi-blaster/pi-blaster_git.bb > | 3 + > 3 files changed, 110 insertions(+) > create mode 100644 recipes-devtools/pi-blaster/files/initscript.patch > create mode 100644 recipes-devtools/pi-blaster/pi-blaster.inc > create mode 100644 recipes-devtools/pi-blaster/pi-blaster_git.bb > > > > You missed the actual patch so I will give my feedback listed below: > 1. Refactor commit log. > 2. One space left after "oe_runmake". > 3. Why exactly do you need oe_runmake after all? > 4. I would replace the do install/configure appends with a patch on > Makefile (would be useful for the project's maintainer too). > 5. Please add comments to patches too. Here is an example: > http://git.yoctoproject.org/cgit/cgit.cgi/meta-raspberrypi/tree/recipes-bsp/rpi-mkimage/rpi-mkimage/open-files-relative-to-script.patch > Thanks for the comprehensive response Andrei, and also for the link to the OpenEmbedded Commit Patch Message Guidelines - very useful. I've reworked the pi-blaster recipe, hopefully in line with your comments, and pushed a second patch-set up to the review server. Regards, Alex --------------070804090807080901090900 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 8bit
On 10/06/2014 20:50, Andrei Gherzan wrote:
Hi Alex,


On Tue, May 20, 2014 at 5:40 PM, Alex J Lennon <ajlennon@dynamicdevices.co.uk> wrote:
Please see following patch for details.

The following changes since commit f3a8693f08f99893453fd1fe282515b2f222c080:

  omxplayer: Update to remote's HEAD (2014-05-09 14:56:59 +0300)

are available in the git repository at:

  git://github.com/DynamicDevices/meta-raspberrypi ajl/pi-blaster
  https://github.com/DynamicDevices/meta-raspberrypi/tree/master

Alex J Lennon (1):
  pi-blaster: Added recipe

 recipes-devtools/pi-blaster/files/initscript.patch | 71 ++++++++++++++++++++++
 recipes-devtools/pi-blaster/pi-blaster.inc         | 36 +++++++++++
 recipes-devtools/pi-blaster/pi-blaster_git.bb      |  3 +
 3 files changed, 110 insertions(+)
 create mode 100644 recipes-devtools/pi-blaster/files/initscript.patch
 create mode 100644 recipes-devtools/pi-blaster/pi-blaster.inc
 create mode 100644 recipes-devtools/pi-blaster/pi-blaster_git.bb

You missed the actual patch so I will give my feedback listed below:
1. Refactor commit log.
2. One space left after "oe_runmake".
3. Why exactly do you need oe_runmake after all?
4. I would replace the do install/configure appends with a patch on Makefile (would be useful for the project's maintainer too).
5. Please add comments to patches too. Here is an example:


Thanks for the comprehensive response Andrei, and also for the link to the OpenEmbedded Commit Patch Message Guidelines - very useful.

I've reworked the pi-blaster recipe, hopefully in line with your comments, and pushed a second patch-set up to the review server.

Regards,

Alex

--------------070804090807080901090900--