Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 1275009 - Review Request: nodejs-chroma-js - JavaScript library for color conversions
Summary: Review Request: nodejs-chroma-js - JavaScript library for color conversions
Keywords:
Status: CLOSED NOTABUG
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Piotr Popieluch
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: nodejs-reviews 1274930
TreeView+ depends on / blocked
 
Reported: 2015-10-24 20:30 UTC by William Moreno
Modified: 2017-02-18 16:15 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2017-02-18 16:15:34 UTC
Type: ---
Embargoed:
piotr1212: fedora-review-

Attachments (Terms of Use)

Description William Moreno 2015-10-24 20:30:52 UTC 
Spec URL: https://williamjmorenor.fedorapeople.org/rpmdev/nodejs-chroma-js.spec
SRPM URL: https://williamjmorenor.fedorapeople.org/rpmdev/nodejs-chroma-js-1.1.1-1.fc24.src.rpm
Description: JavaScript library for color conversions
Fedora Account System Username: williamjmorenor
Comment 1 Piotr Popieluch 2015-11-14 12:40:20 UTC 
Thanks for packaging. 

This needs some work before I can approve:

- License: should be BSD, not GPL, it also seems that there are more license files, please check if this is multi-licensed and set correct licenses
- URL: Please set correct url which points to the actual package
- BuildRequires, I expect there are some deps missing, at least grunt-cli
- %prep, use "cp -p" to preserve timestamps
- %build, package needs to be build with "grunt"
- Please add %check section with tests
- %install, nothing is being installed
- %changelog, incorrect version 1.0.0 instead of 1.1.1

If you need help/clarification on some items let me know.
Comment 2 William Moreno 2015-12-28 22:47:15 UTC 
Thanks for take the review.

I am not sure about how to build the sources with grunt in %%build. Can please provide me a example.
Comment 3 Piotr Popieluch 2016-01-28 10:29:56 UTC 
Excuse me for the late response

you can build with:


BuildRequires: npm(grunt-cli)
BuildRequires: npm(grunt-contrib-clean)
BuildRequires: npm(grunt-contrib-coffee)
BuildRequires: npm(grunt-replace)
BuildRequires: npm(grunt-contrib-uglify)

%build
%nodejs_symlink_deps --build
grunt


Only thing is that dependency grunt-replace is not packaged yet. You will have to package that one first (or just use sed to set the version in chroma.js, and patch out grunt-replace out of the Gruntfile.js).
Comment 4 Piotr Popieluch 2017-02-18 12:52:02 UTC 
This is a quite old review request, do you still want to package this or can we close this request?
Comment 5 William Moreno 2017-02-18 16:15:34 UTC 
Not rigth now, thanks for the review.
Note You need to log in before you can comment on or make changes to this bug.