[Pkg-javascript-devel] Bug#890539: RFS: node-yamljs/0.3.0+dfsg-1 [ITP]

[Bastien ROUCARIES]

On Mon, May 14, 2018 at 8:32 PM, Andreas Moog <andreas.moog at warperbbs.de> wrote:
> On Sun, May 13, 2018 at 09:20:40PM +0200, Bastien ROUCARIES wrote:
>
> Thanks for the review. I have a couple of questions about some of the points:
>
>> 1. you need a smoke test : aka run a json file and convert to yaml.
>> create a debian/smoketest dir and put file here
>
> 1. Ok, I added 2 more tests to debian/tests, one for yaml2json, one for
> json2yaml.

Not good from security point of view, you need to use mktemp.

And diff does not fail if content change. Better to use cmp tool

>> 2. why did you not run testsuite ?
>
> As far as I can tell the testsuite requires node-jasmine, which isn't packaged
> for Debian.

Ok please create a itp or rfp for nde-jasmine and mention this
package. Will sponsor node-jasmine

in all the case add to makefile (this time no vulnerability because
you write to local build):
override_dh_auto_test:

%.yaml: %.json


ifeq (,$(filter nocheck,$(DEB_BUILD_OPTIONS) $(DEB_BUILD_PROFILES)))
    mkdir -p debian/testsuite/data.
    yaml2json debian/tests/data/yaml.test > debian/testsuite/
else
    @echo '**********************************************************'
    @echo 'Skip test suite                                           '
    @echo '**********************************************************'
endif


>
>> 3 copyright is not good run on github tree
>> git log gihtub/master  --format="%an <%ae>" |  sort -u
>
> That gives me a list of about 20 contributors, some with only very minor
> contributions like a typo fix. Where would I make the cutoff point for
> including them in debian/copyright? Adding all of the contributors feels
> wrong. The LICENSE only mentions jeremyfa as copyright holder.

Better safe then sorry. Please do it

>> 4. copyright year are 2010-2017
>
> Fixed.
>
>> 5 install is not right you need only to install index.js and lib/ cli/
>> package.json files under nodes dir
>
> Right, I also added the bin dir since the 2 standalone scripts use only
> relative paths to find the libraries (via "var YAML=require('../lib/Yaml.js')")
> and I didn't want to diverge from upstream here.
>
>> 6. you should also exclude lib if you rebuilt it under copyright file-excluded:
>
> Good point, done.
>
>> 7. demo.html should be ship with dh_examples
>
> It already is in debian/examples, do you mean I should put it somewhere else?
>
> I pushed an updated repository with the current state of my work.
>
> Viele Grüße // Kind Regards
>
> Andreas
>
> --
> PGP-encrypted mails preferred
> PGP Fingerprint: 74CD D9FE 5BCB FE0D 13EE 8EEA 61F3 4426 74DE 6624