I'll review this

Few points before I start complete review:
 * package doesn't build in mock. Please verify packages in mock before submitting reviews
 * 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?)
 * Why would "cp -pr %{nodejs_sitelib} ." in check be needed?
 * 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)

(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

new versions of spec file and srpm

Spec URL: http://fedorapeople.org/~humaton/nodejs-uglify-js.spec
SRPM URL: http://fedorapeople.org/~humaton/nodejs-uglify-js-2.2.5-2.fc19.src.rpm

I've sponsored Tomas now, package looks good. I'll be mentoring him in coming weeks.

automatic requires look OK, licensing is OK, tests pass...APPROVED

New Package SCM Request
=======================
Package Name: nodejs-uglify-js
Short Description: JavaScript parser/compressor/beautifier
Owners: humaton
Branches: f18 f19 f20 el6
InitialCC:

Git done (by process-git-requests).

Guys, what is going on here? It seems to be duplicate of [1]. I would suggest to retire the package immediately, prior pushing any updates.


[1] https://bugzilla.redhat.com/show_bug.cgi?id=894725

Now the question is...why is original package not following the same naming rules as almost every other nodejs package? Adding T.C. to CC so we can some answer :-) Perhaps let's make this a package rename then? With proper obsoletes/provides?

It is not nodejs package. It is not using any nodejs specific functionality. You should be able to run it by any JS engine available on your system. Hence it is without prefix.

There seems to be tendency to believe that every JS package is nodejs package, but that is not true. I am afraid there is more nodejs- packages which shouldn't have that prefix (but I have no evidence of such packages, yet :).

(In reply to comment #9)
> Now the question is...why is original package not following the same naming
> rules as almost every other nodejs package?

https://fedoraproject.org/wiki/Packaging:Node.js#Naming_Guidelines

> Application packages that mainly provide tools (as opposed to libraries) that happen to be written for Node.js must follow the general naming guidelines instead.

Lots of people just use /usr/bin/uglifyjs, so I didn't prefix it.  Splitting the CLI and the library didn't seem to make much sense, since they'd still both depend on node.

(In reply to comment #0)
> It is developed on NodeJS, but it should work on any JavaScript platform
> supporting the CommonJS
> module system.

Note that we carry a pure-JS version (e.g. doesn't require a CommonJS interpreter; Ruby uses it) in the uglify-js-common package.

Very well that actually makes sense. My bad. 

Tomas can you please go through http://fedoraproject.org/wiki/How_to_remove_a_package_at_end_of_life process? It's a bummer but oh well...

package retired

*** This bug has been marked as a duplicate of bug 894725 ***