Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=951360

--- Comment #3 from Tomas Hrcka <[email protected]> ---
(In reply to comment #2)
> Few points before I start complete review:
>  * package doesn't build in mock. Please verify packages in mock before
> submitting reviews

I will.

>  * Requires on nodejs and npm are most likely bogus. Requires on nodejs
> engine is generated automatically, and npm is most likely not needed for
> package to work correctly (or is it?)

You are right removing them from spec file

>  * Why would "cp -pr %{nodejs_sitelib} ." in check be needed?

Because npm modules are linked to the root dir of the module. Otherwise tests
will not run because of missing dependencies.

>  * Just for the record (this doesn't affect the review but FYI) following
> parts 
>    are not needed in Fedora/EL6+:
>     * rm -rf %{buildroot}
>     * whole %clean section
>     * %defattr(-,root,root,-) (unless you really want to change defaults)

Going to update spec and re-upload

-- 
You are receiving this mail because:
You are on the CC list for the bug.
Unsubscribe from this bug 
https://bugzilla.redhat.com/token.cgi?t=QZllyYumZJ&a=cc_unsubscribe
_______________________________________________
package-review mailing list
[email protected]
https://admin.fedoraproject.org/mailman/listinfo/package-review

Reply via email to