Bug 921365 - lessjs does not comply to the Node.js Packaging Guidelines
Summary: lessjs does not comply to the Node.js Packaging Guidelines
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: lessjs
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: T.C. Hollingsworth
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
: 921932 (view as bug list)
Depends On: 921847
Blocks:
TreeView+ depends on / blocked
 
Reported: 2013-03-14 04:23 UTC by T.C. Hollingsworth
Modified: 2013-04-19 01:12 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2013-04-19 01:12:10 UTC
Type: Bug
Embargoed:


Attachments (Terms of Use)

Description T.C. Hollingsworth 2013-03-14 04:23:55 UTC
https://fedoraproject.org/wiki/Packaging:Node.js

This prevents less from being used programatically from other node programs.

I requested ACLs if you don't mind me fixing it up myself. ;-)
I'd also like to look at packaging the necessary YUI bits so we can turn them on, if that's possible.

Comment 1 Stephen Gallagher 2013-03-14 11:37:15 UTC
I'm thinking that we may actually want to do a rename and re-review of lessjs so that it follows the packaging guidelines (including naming). It pre-dated the creation of those guidelines, but it should pretty clearly be named nodejs-lessjs.

There were several reasons I disabled the YUI compression. The first and most important one was that less.js was carrying a bundled copy of cssmin.js and I didn't have an obvious way to eliminate that at the time. This might not be an issue in the latest versions; the release notes from 1.3.2 states: "change dependency on cssmin to ycssmin". Needs investigation.

If I remember correctly, I avoided packaging YUI primarily because it was mostly Java and I don't consider myself qualified to handle that. If you are willing to take that on, you certainly have my blessing.

I've granted youu ACLs on lessjs if you want to make changes there, but I think the rename is probably worthwhile, so if you submit a new package review, I'll perform the review and comaintain with you.

Comment 2 Stephen Gallagher 2013-03-14 11:38:16 UTC
Actually, looks like mrunge granted you the ACLs before I got there. Works for me :)

Comment 3 Matthias Runge 2013-03-14 11:48:46 UTC
yes, less.js was introduced a longer time ago. (it feels like a long time, actually). If it's more compliant with our Packaging guidelines, please rename the package; I can do the review, if there's a helping hand required.

If I remember right, I contacted upstream regarding cssmin. They didn't see a reason not to switch to yui compressor. Also they didn't use cssmin any more.

Stephen, I think you actually disabled the compressor in less.js; until now, nobody complained about it, but I might be wrong here.

Comment 4 Stephen Gallagher 2013-03-14 11:53:48 UTC
Matthias, I disabled the *yui* compressor (and have the binary returning a "not supported" error when you try to use it), but I left the trivial compressor (just eliminates non-essential whitespace) available.

I'm fine with re-enabling it if we can do so now, but at the time I was on a deadline and avoiding the bundling was more important than trying to package the YUI compressor and its java (which I don't remember all the details about, but may also have been bundling).

Comment 5 T.C. Hollingsworth 2013-03-14 23:07:22 UTC
(In reply to comment #1)
> I'm thinking that we may actually want to do a rename and re-review of
> lessjs so that it follows the packaging guidelines (including naming). It
> pre-dated the creation of those guidelines, but it should pretty clearly be
> named nodejs-lessjs.

I disagree.  I think this falls under the "tool that happens to be written with node" side of the Naming Guidelines rather than the library side.  Yeah, web frameworks and the like can use it programatically to provide convienence, but the vast majority of users in production compile their less to CSS up-front with lessc.

The rule of thumb I use when deciding whether to apply language-specific library prefixes in Fedora (whether it be "nodejs-", "python-", whathaveyou) is if you use the thing in %{_bindir} more than the library (e.g. via require() in node), then don't worry about the prefix.  I think this is the case for less.

> There were several reasons I disabled the YUI compression. The first and
> most important one was that less.js was carrying a bundled copy of cssmin.js
> and I didn't have an obvious way to eliminate that at the time. This might
> not be an issue in the latest versions; the release notes from 1.3.2 states:
> "change dependency on cssmin to ycssmin". Needs investigation.
> 
> If I remember correctly, I avoided packaging YUI primarily because it was
> mostly Java and I don't consider myself qualified to handle that. If you are
> willing to take that on, you certainly have my blessing.

Ugh, I thought the YUI bits involved were part of the regular YUI JS library.  I guess the YUI compressor it's own beast, and an ugly one at that.

It looks like it bundles Rhino.  Leaving aside that, mcepl looked into Rhino back when were were trying to get node in the first time around and its bundled library situation made node's look downright tame.  I'd have to take another look to be sure but I doubt this is acceptable for Fedora.  :-(

Comment 6 T.C. Hollingsworth 2013-03-15 06:18:22 UTC
Now that I've had a look at this I see ycssmin is a Yahoo!-maintained fork of the CSS minification code from YUI Compressor that works with node instead of Rhino, so it should be all good.  The review is in bug 921847.

Comment 7 Matthias Runge 2013-03-15 10:15:25 UTC
*** Bug 921932 has been marked as a duplicate of this bug. ***

Comment 8 T.C. Hollingsworth 2013-04-19 01:12:10 UTC
Fixed during the nodejs-less rename.


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