Bug#622628: [Pkg-javascript-devel] Bug#622628: npm ready to be reviewed

2012-04-06 Thread Jérémy Lal
On 05/04/2012 14:01, Jonas Smedegaard wrote:
 On 12-04-03 at 11:57pm, Jérémy Lal wrote:
 it would be very nice to review npm package sitting at : 
 git://anonscm.debian.org/git/collab-maint/npm.git
 
 
 Patch 2001 refer to debian/copyright.  I recommend to document 
 explicitly, as the patch file will appear outside of the context of the 
 full packaging - i.e. at http://patch-tracker.debian.org/package/npm
 
 Maybe add a brief explicit note and then refer (with a URL, not a 
 relative file reference) to further explanation e.g. a post to a 
 bugreport.

I added a link to upstream version of the license (the actual commit),
and a short explanation. Reading the license is clear and fast enough to
understand why.


 I recommend to use ~dfsg (not ~dfsg9) in package versioning.  Remember 
 to update everywhere, also e.g. in NEWS file.

Done...

 
 Does not seem like news me to warn against use as root - and therefore 
 inappropriate to list in NEWS file.  The similar text in README.Debian 
 is vague: first a feature is described, and only in next separate 
 pragraph discouraged.

Ok, NEWS states only what is broken and where are the docs.
README.Debian states what is special to debian.

 
 Please avoid versioned (build-)dependencies when required version is 
 satisfied in all Debian distros releases where the package is available 
 at all.

Yep.


 Feels odd to me that Node is explained at the end of long description. I 
 suggest to first introduce Node and afterwards go into more details.

Damn I did it like that for all other node-* packages.
Fixed here.


 Are you sure it is necessary to set the bash-completion script 
 executable?  Seems odd to me that the dh_bash-completion script wouldn't 
 take care of that if really needed.

Old error, fixed but needed a patch to remove shebang.
 
 Please use either true upstream URL (at Github) or a Debian-maintained 
 redirection service to track and download upstream source (see node-xmpp 
 for an example, using githubredir.debian.net).  The npmjs.org registry 
 is nice but less trustworthy.

Lot more work to do, but done, see git log (i hope i managed to get
something readable this time).


 Repackaging of upstream source should be mentioned in Source paragraph 
 in debian/copyright.  I recommend to also add a list of files/dirs 
 stripped in an unofficial Files-Excluded paragraph (I intend to propose 
 that as a future extension to DEP5 copyright file format, and also to 
 make use of it in CDBS at some point).  See ghostscript packaging for an 
 example.

Seems nice, done.
Still copyright-format-1.0 since extra fields are allowed.


 You should not rely on executable bit being properly set in sources.  So 
 instead of executing ./configure I suggest to invoke bash ./configure.  
 Or even better: Ship a prepared npmrc in debian subdir to avoid the need 
 to execute upstream source during build (which is a slight security 
 risk).

Fixed.
I wonder why i even did it like that. Influenced by upstream, maybe :)

NB: Should i list added Build-Depends in changelog ?

Jérémy.




--
To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#622628: [Pkg-javascript-devel] Bug#622628: npm ready to be reviewed

2012-04-05 Thread Jonas Smedegaard
On 12-04-03 at 11:57pm, Jérémy Lal wrote:
 it would be very nice to review npm package sitting at : 
 git://anonscm.debian.org/git/collab-maint/npm.git


Patch 2001 refer to debian/copyright.  I recommend to document 
explicitly, as the patch file will appear outside of the context of the 
full packaging - i.e. at http://patch-tracker.debian.org/package/npm

Maybe add a brief explicit note and then refer (with a URL, not a 
relative file reference) to further explanation e.g. a post to a 
bugreport.


I recommend to use ~dfsg (not ~dfsg9) in package versioning.  Remember 
to update everywhere, also e.g. in NEWS file.


Does not seem like news me to warn against use as root - and therefore 
inappropriate to list in NEWS file.  The similar text in README.Debian 
is vague: first a feature is described, and only in next separate 
pragraph discouraged.


Please avoid versioned (build-)dependencies when required version is 
satisfied in all Debian distros releases where the package is available 
at all.


Feels odd to me that Node is explained at the end of long description. I 
suggest to first introduce Node and afterwards go into more details.


Are you sure it is necessary to set the bash-completion script 
executable?  Seems odd to me that the dh_bash-completion script wouldn't 
take care of that if really needed.


Please use either true upstream URL (at Github) or a Debian-maintained 
redirection service to track and download upstream source (see node-xmpp 
for an example, using githubredir.debian.net).  The npmjs.org registry 
is nice but less trustworthy.


Repackaging of upstream source should be mentioned in Source paragraph 
in debian/copyright.  I recommend to also add a list of files/dirs 
stripped in an unofficial Files-Excluded paragraph (I intend to propose 
that as a future extension to DEP5 copyright file format, and also to 
make use of it in CDBS at some point).  See ghostscript packaging for an 
example.


You should not rely on executable bit being properly set in sources.  So 
instead of executing ./configure I suggest to invoke bash ./configure.  
Or even better: Ship a prepared npmrc in debian subdir to avoid the need 
to execute upstream source during build (which is a slight security 
risk).


 - Jonas

-- 
 * Jonas Smedegaard - idealist  Internet-arkitekt
 * Tlf.: +45 40843136  Website: http://dr.jones.dk/

 [x] quote me freely  [ ] ask before reusing  [ ] keep private


signature.asc
Description: Digital signature