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