[Pkg-javascript-devel] Please review libjs-jquery-coolfieldset

[Emilien Klein]

Hi François-Régis,

2014-03-07 17:33 GMT+01:00 François-Régis <frv-debian at miradou.com>:
> Hi everybody,
>
> Could anybody have a look at libjs-jquery-coolfieldset [1], it's my
> first js package so please look it closely...

I've looked at your package (not tried to build it), here are a couple
of comments:

* d/copyright:
  - the copyright of each file in the upstream tarball (even if it's
not present in the Debian binary package) must be mentioned. jQuery
(file js/jquery.js) must be mentioned.
  - You are not specifying the copyright of the files in the Debian
package. I expect those to be under your copyright, possibly using the
same license as upstream?
  - Upstream-Name: You've specified the same name as the Debian
package, but I don't see that name back on
http://w3shaman.com/article/jquery-plugin-collapsible-fieldset .
Looking at the name of the upstream tarball and the line "By the way,
I named this plugin coolfieldset." I'd guess it should be
"coolfieldset".
  - Question about Upstream-Contact: Should it be Luc Ky, or (as
specified in the copyright mention in file js/jquery.coolfieldset.js:
"Lucky"?

* d/control:
  - About the name of the package: Looking at the packages maintained
in our team's Git repo [0], most source packages have a name that
doesn't include "libjs-", while the binary package does. I'm wondering
if your source package should not just be named "jquery-coolfieldset",
while the source package remains "libjs-jquery-coolfieldset"?
  - Homepage: You link to the .js file on Google Code. Wouldn't this
be a better homepage, as it contains examples and all:
http://w3shaman.com/article/jquery-plugin-collapsible-fieldset
  - The Vcs-Browser link returns a 404 error (hint: not in collab-maint)
  - Same issue for your Vcs-Git link
  - 2 typo in the Description: "This plugin can collapsed or hide
fieldset" -> "This plugin can collapse or hide fieldsets"
  - 3 other typo: "The separated CSS file will also useful if you need
to modify the fieldset appearance." -> "The separate CSS file will
also be useful if you need to modify the fieldset's appearance."
  - In my js package [1], I've added "Suggests: javascript-common".
Can't remember if that was a suggestion, or if I saw that in some
other Debian js package. Please investigate if this would provide any
benefit to the user.

* d/rules:
  - you use uglifyjs, but it's not indicated in d/control's
Build-Depends (node-uglify)
  - I find it a bit suprising that the build directory is in the root,
not the debian directory. Is that on purpose? (my packages build
inside a debian/tmp-build folder, so as to not change the upstream
structure more than adding the debian folder). I don't know if there
is a strong Debian policy on that, input from others on the team
welcome.
  - Why are you deleting js/*.min.js in override_dh_auto_clean?
Looking at your call to uglifyjs, it looks like only one minified file
will be created, in the build folder (not js folder)?

* d/README.source: read the content of the file, and take appropriate action

* d/docs: empty file?

* d/install: see comment about build folder in d/rules

* d/watch:
  - A Github repository is mentioned, that seems to be a fork. But
that is not mentioned anywhere (namely in d/copyright, d/control and
d/README.source)
  - Also, if you plan to look at Github from your watchfile, please
use the Debian githubredir service.


Hope this helps. Let us know on the list if you have questions, or
don't agree with my comments.
Thanks for your packaging efforts.
    +Emilien

[0] http://anonscm.debian.org/gitweb/?a=project_list;pf=pkg-javascript
[1] http://anonscm.debian.org/gitweb/?p=pkg-javascript/jquery-lazyload.git;a=blob_plain;f=debian/control;hb=HEAD

>
> Any comment, suggestion or insult are welcome ;=)
>
> [1]
> http://anonscm.debian.org/gitweb/?p=pkg-javascript/libjs-jquery-coolfieldset.git
>
> Thank you.
>
> Regards,
>
> --
> François-Régis
>
> _______________________________________________
> Pkg-javascript-devel mailing list
> Pkg-javascript-devel at lists.alioth.debian.org
> http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-javascript-devel