Review of lightread

[Stéphane Graber]

On 08/06/2012 09:39 AM, Stéphane Graber wrote:
> 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

Might be worth mentioning that I haven't actually installed or ran the
package, just did the usual source and binary package analysis.

Unless I missed some e-mails, we're currently at a score of +1 on this
one (-1 from Allison, +1 from Bhavani and +1 from me).

Allison: It'd be great if you could review that one again and check that
it works now.


-- 
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/c39b62e6/attachment.pgp>