Review of lightread

Didier Roche didrocks at ubuntu.com
Mon Aug 6 13:48:01 UTC 2012


Le 06/08/2012 15:39, Stéphane Graber a écrit :
> 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)
What should it be updated to? (the version seems to make sense to me)
>   - 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 I've already the above branch prepared, I've removed the executable 
bits (hard to spot as /opt is not supported at all by lintian and we 
already get tons of lintian warnings because of the platform which has 
never been prepared to /opt…)
>   - 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.

This hook is created automatically by dh_python. It's useless, right, 
with /opt as nobody before taking the decision of mandating /opt made 
sure the platform was prepared with it. Apart from having an even more 
hackish debian/rules, I don't think we should bother about that one.

changes pushed to my previous url.
Cheers,
Didier



More information about the App-review-board mailing list