Bug 1171263 - Review Request: nodejs-istanbul - A JS code coverage tool written in JS
Summary: Review Request: nodejs-istanbul - A JS code coverage tool written in JS
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Parag AN(पराग)
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: 1123537 1171221 1171403
Blocks: 1171302 1171303 1171306 1171309 1171311 1171314 1171320 1171323 1171327 1171334 1171335 1171336 1171338 1171345 1171350
TreeView+ depends on / blocked
 
Reported: 2014-12-05 18:56 UTC by Piotr Popieluch
Modified: 2015-09-27 18:22 UTC (History)
2 users (show)

Fixed In Version: nodejs-istanbul-0.3.2-3.fc21
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2014-12-19 20:04:45 UTC
panemade: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Piotr Popieluch 2014-12-05 18:56:45 UTC
Spec URL: https://piotrp.fedorapeople.org/nodejs-istanbul.spec
SRPM URL: https://piotrp.fedorapeople.org/nodejs-istanbul-0.3.2-1.fc21.src.rpm
Description: A JS code coverage tool written in JS
Fedora Account System Username: piotrp

Comment 1 Parag AN(पराग) 2014-12-06 12:49:54 UTC
Review:

+ Package built successful in mock (f22 x86_64)

+ rpmlint on generated rpms gave output
nodejs-entities.noarch: W: only-non-binary-in-usr-lib
2 packages and 0 specfiles checked; 0 errors, 1 warnings.

+ Source verified with upstream as (sha256sum)
srpm tarball: 1118cdb833068b6450d48946fb53cece6e41a3e95b29d24871898cb7a71ffced
upstream tarball:  1118cdb833068b6450d48946fb53cece6e41a3e95b29d24871898cb7a71ffced

- License is "BSD" and not "MIT. 

+ License text is included in its own LICENSE file.

+ follows nodejs packaging guidelines.

Suggestions:
1) Group tag is optional, can be removed.

2) Fix license tag

Comment 2 Parag AN(पराग) 2014-12-06 13:23:59 UTC
oops! my bad. I have many tabs open and I wrote wrong review above. 
Here is correct one

Review:
- Package built successful in mock (f22 x86_64)

+ Source istanbul-0.3.2.tgz verified with upstream as (sha256sum)
srpm tarball: 95f3d546e15b110ea75466058c6247d1fb940771ae640eb49d02ad124995920d
upstream tarball:  95f3d546e15b110ea75466058c6247d1fb940771ae640eb49d02ad124995920d

+ License is "BSD" and included in its own LICENSE file.

+ follows nodejs packaging guidelines.

Suggestions:
1) Group tag is optional, can be removed.

2) tests are failing with error
Cannot find module '../dist/cjs/handlebars'
Error: Cannot find module '../dist/cjs/handlebars'

Please fix the tests or disable them by filing upstream issue with using tests.

Comment 3 Piotr Popieluch 2014-12-06 15:05:16 UTC
Seems like a bug in nodejs-handlebars, tests did succeed here (with my variant of handlebars)

Comment 4 Fedora Update System 2014-12-09 01:07:30 UTC
nodejs-handlebars-2.0.0-3.fc21 has been submitted as an update for Fedora 21.
https://admin.fedoraproject.org/updates/nodejs-handlebars-2.0.0-3.fc21

Comment 5 Parag AN(पराग) 2014-12-09 06:00:52 UTC
I tried above handlebars update but for me local build still complains same
Cannot find module '../dist/cjs/handlebars'
Error: Cannot find module '../dist/cjs/handlebars'

Comment 6 Parag AN(पराग) 2014-12-09 06:04:57 UTC
Ah and after disabling tests in nodejs-istanbul.spec, building it, I got a file name conflict
Error: Transaction check error:
  file /usr/bin/istanbul from install of nodejs-istanbul-0.3.2-1.fc21.noarch conflicts with file from package istanbul-0.2.2-22.fc21.x86_64

Maybe you should install it as istanbul-js

Comment 7 Parag AN(पराग) 2014-12-10 08:08:37 UTC
I found packaging issue with nodejs-handlebars and reported in bug 1172471.

Comment 8 Piotr Popieluch 2014-12-10 08:30:08 UTC
Thank you. I was actually looking into it right now, you beat me ;)

updated spec and srpm:
Spec URL: https://piotrp.fedorapeople.org/nodejs-istanbul.spec
SRPM URL: https://piotrp.fedorapeople.org/nodejs-istanbul-0.3.2-3.fc21.src.rpm


- Renamed binary name to prevent conflict
- Temporary disabled tests because of RHBZ#1172471
- Removed group tag
- Added rm -rf node_modules


Only actually using istanbul gives me the same error as the test because of missing dist dir:


➜  rpmbuild  /usr/bin/istanbul-js 
Cannot find module '../dist/cjs/handlebars'
Error: Cannot find module '../dist/cjs/handlebars'
    at Function.Module._resolveFilename (module.js:338:15)
    at Function.Module._load (module.js:280:25)
    at Module.require (module.js:364:17)
    at require (module.js:380:17)
    at Object.<anonymous> (/usr/lib/node_modules/handlebars/index.js:6:18)
    at Module._compile (module.js:456:26)
    at Object.Module._extensions..js (module.js:474:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:312:12)
    at Module.require (module.js:364:17)

Error: Could not register report from file html.js
    at /usr/lib/node_modules/istanbul/lib/util/factory.js:60:27
    at Array.forEach (native)
    at Object.Factory.loadStandard (/usr/lib/node_modules/istanbul/lib/util/factory.js:53:29)
    at Object.<anonymous> (/usr/lib/node_modules/istanbul/lib/register-plugins.js:11:8)
    at Module._compile (module.js:456:26)
    at Object.Module._extensions..js (module.js:474:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:312:12)
    at Module.require (module.js:364:17)
    at require (module.js:380:17)



Should we wait on https://bugzilla.redhat.com/show_bug.cgi?id=1172471 before approving?

Comment 9 Parag AN(पराग) 2014-12-10 09:07:38 UTC
I don't think we need to wait for bug 1172471 and also I need this package badly for some of my upcoming nodejs package reviews.

APPROVED.

Comment 10 Piotr Popieluch 2014-12-10 10:20:04 UTC
New Package SCM Request
=======================
Package Name: nodejs-istanbul
Short Description: A JS code coverage tool written in JS
Upstream URL: https://github.com/gotwarlost/istanbul
Owners: piotrp
Branches: f19 f20 f21 el6 epel7

Comment 11 Gwyn Ciesla 2014-12-10 15:10:31 UTC
Git done (by process-git-requests).

Comment 12 Fedora Update System 2014-12-13 15:30:43 UTC
nodejs-istanbul-0.3.2-3.fc21 has been submitted as an update for Fedora 21.
https://admin.fedoraproject.org/updates/nodejs-istanbul-0.3.2-3.fc21

Comment 13 Fedora Update System 2014-12-13 15:31:17 UTC
nodejs-istanbul-0.3.2-3.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/nodejs-istanbul-0.3.2-3.fc20

Comment 14 Fedora Update System 2014-12-13 15:33:25 UTC
nodejs-istanbul-0.3.2-3.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/nodejs-istanbul-0.3.2-3.fc19

Comment 15 Fedora Update System 2014-12-13 15:34:01 UTC
nodejs-istanbul-0.3.2-3.el7 has been submitted as an update for Fedora EPEL 7.
https://admin.fedoraproject.org/updates/nodejs-istanbul-0.3.2-3.el7

Comment 16 Fedora Update System 2014-12-14 22:06:41 UTC
nodejs-istanbul-0.3.2-3.el7 has been pushed to the Fedora EPEL 7 testing repository.

Comment 17 Fedora Update System 2014-12-23 18:25:49 UTC
nodejs-istanbul-0.3.2-3.fc20 has been pushed to the Fedora 20 stable repository.

Comment 18 Fedora Update System 2014-12-23 18:26:24 UTC
nodejs-istanbul-0.3.2-3.fc21 has been pushed to the Fedora 21 stable repository.

Comment 19 Fedora Update System 2015-09-27 18:22:15 UTC
nodejs-istanbul-0.3.2-3.el7 has been pushed to the Fedora EPEL 7 stable repository. If problems still persist, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.