On 10/06/2014 20:50, Andrei Gherzan wrote: > Hi Alex, > > > On Tue, May 20, 2014 at 5:40 PM, Alex J Lennon > <[email protected] <mailto:[email protected]>> > 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 > <http://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 > <http://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 > <http://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
-- _______________________________________________ yocto mailing list [email protected] https://lists.yoctoproject.org/listinfo/yocto
