Review of lightread

[Stéphane Graber]

On 08/06/2012 09:23 AM, Didier Roche wrote:
> Hey guys,
> 
> As dpm asked me, I did a review of lightread source code and packaging
> themselves.
> 
> The code is cleaned, using sass and some popular javascript libraries,
> clearly showing the guy know what he is doing. I just removed an
> harmless .DS_Store which isn't useful in this context, as well as a
> readabilty.js unsued file. Even if it's unused, the goal of this file is
> to use a third-party service to prettify the blog entry. There is a
> token and so on. However, this one isn't even called, so I felt better
> to completely remove it (which is just about removing that file right now).
> 
> https://code.launchpad.net/~didrocks/ubuntu-app-reviews/lightread
> 
> The packaging itself works from /opt and comply the standards of the
> ARB. So a definitive +1 from me.
> 
> Cheers,
> Didier

Just spent 5 minutes reviewing it here too.

 - debian/changelog will need update prior to upload (version number)
 - lintian spotted two small problems:
W: lightread: executable-not-elf-or-script
opt/extras.ubuntu.com/lightread/share/lightread/media/app/scripts/lib/keymaster.js
W: lightread: executable-not-elf-or-script
opt/extras.ubuntu.com/lightread/share/lightread/media/app/manifest.json
 - As already mentioned, the package ships a runtime.d directory but
without the usual hook in there. The hook should be included or the
directory removed.

These seem fairly easy to get fixed so ideally they should, but I'm not
going to block on that. +1


-- 
Stéphane Graber
Ubuntu developer
http://www.ubuntu.com

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 900 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/app-review-board/attachments/20120806/2138be50/attachment.pgp>