[Pkg-javascript-devel] review of jquery-lazyload version 1.9.3

[Emilien Klein]

Hi Jeremy and team,
2014-09-27 14:34 GMT+02:00 Emilien Klein <emilien+debian at klein.st>:
> Hi Jeremy and team,
>
> 2014-09-09 22:54 GMT+02:00 Jérémy Lal <kapouer at melix.org>:
>> Hi,
>>
>> * first you need to learn how to properly repackage an upstream tarball,
>> because the "good" way is so easy and fun !
>> Just add a Files-Excluded section in debian/copyright as explained in
>> https://wiki.debian.org/UscanEnhancements
>> then run
>> uscan --force-download
>> and voilà - a nice repackaged tarball.
>> Then it's only a matter of renaming it (sometimes it doesn't add +dfsg
>> to the upstream version in the tarball file name), and
>> git-import-orig ../thattarball_1.2.3+dfsg.orig.tar.gz
>
> Yes, that is indeed much cleaner.
> As part of my involvement with the Debian Med team, Andreas Tille has
> made me aware of this nice mechanism (Andreas pushed for this, see
> #685787).
> Since the 1.9.3 version is already in the git repo, it doesn't make
> much sense to modify this now, but I will transition to Files-Excluded
> when the next upstream version is released. That will also allow to
> remove d/download_repackage_dfsg.sh
>
>> * do not call uupdate in uscan, it gets in the way of git-buildpackage
>
> Done.
>
>> * note that directly modifying upstream files manually in a
>> git-buildpackage-maintained repository is NOT a good idea.
>> You're supposed to modify only debian/ dir, and use git-import-orig for
>> anything else.
>
> There are several branches in this git repo:
> * upstream: the unmodified upstream files, containing e.g. the
> minified js files that get removed in the DFSG-ing process
> * pristine-tar: used to keep the deltas from generated by pristine-tar
> * dfsg_clean: that gets refreshed to the latest upstream version from
> the upstream branch, and then the non-DFSG-approved files get removed.
> After that action, the branch is in the same state as Files-Excluded
> would bring it to, if it was already being used (upstream files, no
> non-DFSG files, no /debian directory)
> * master: that's where the package is build from. It is the content of
> dfsg_clean, and contains the debian repository.
>
> The changes to the debian source package are only performed in the
> master branch.
> Do you have concerns with that for now (until the next release and the
> move to Files-Excluded, which will result in loosing the actual
> untouched upstream content)?
>
>> * debian/README.source is interesting... but anything inside it is
>> already documented on wiki.debian.org or other manuals. Switching to
>> Files-Excluded and uscan allows you to get rid of that file.
>
> As this package doesn't get updated often, I like to keep the full
> instructions in a local text file (it's a reminder to my future self,
> to allow for reproducible builds).
> The one time I felt smart enough to not look at it before packaging
> the new version from a new development machine, I forgot to checkout
> the non-master branches, which resulted in conflicts when pushing
> upstream. That forced me to create a new repo on our Git server, as I
> mentioned in [0].
> I will leave this file for now, and will update it when switching to
> Files-Excluded.
> [0] http://lists.alioth.debian.org/pipermail/pkg-javascript-devel/2014-February/007044.html
>
>> * debian/README.Debian: you should remove everything that is already in
>> README.textile. You should mention javascript-common.
>
> Done.
>
>> * debian/control: recommend javascript-common,
>>   see https://wiki.debian.org/Javascript/Policy
>
> Done (used to be a Suggest, not sure why I removed it in commit 2f93988b)
>
>> * debian/download_repackage_dfsg.sh: no longer needed
>
> Agreed.
> See above, will be removed when the next upstream version is released.
>
>> * debian/rules: README.textile is not a changelog,
>>  simply add debian/docs containing "README.textile"
>
> 70.5% of the lines (86 of 122) are changelog information, hence my
> call to override_dh_installchangelogs.
> But I've changed that and marked it as purely a doc.
>
>> * debian/patches: please use DEP3 patch format. Example:
>> http://anonscm.debian.org/cgit/collab-maint/nodejs.git/tree/debian/patches/1005_restore_sh_javascript_nonminified.patch
>
> Done. Thanks for pointing that out, I was familiar with adding a
> description using `dquilt header -e`, but wasn't familiar about DEP-3
> itself.
>
>
> I've also addressed the new Lintian warning
> W: jquery-lazyload source: space-in-std-shortname-in-dep5-copyright
> cc-by 3.0 (paragraph at line 49)
>
>
>> Thank you for your contribution to Debian !
>
> Thanks for your review.
> I've pushed my changes to pkg-javascript/jquery-lazyload.git on
> Alioth. Let me know if there is anything else you'd like adapted.

Are you OK with my updates?
The one topic that I'm keeping for the next version is transitioning
to Files-Excluded, the rest has been addressed.
Thanks,
   +Emilien