sproutsocial / walltime-js

A JavaScript library for easily translating a UTC time to a "Wall Time" for a particular time zone.
MIT License
121 stars 12 forks source link

Move some deps to devDependencies #47

Closed cheddar closed 10 years ago

cheddar commented 10 years ago

I just pulled this through npm and it ended up pulling coffee script and should as well. I'm assuming they are not runtime dependencies.

I think uglify-js is also a build-time concern, so devDependencies should be sufficient?

I'm not familiar enough with requirejs or how you are using line-reader or strscan to know where those belong, but I'm pretty sure that these 3 don't need to be pulled down into my project that is just trying to use walltime.

jgable commented 10 years ago

Coffeescript is not a dev dependency. It's required to parse the files when the module is required. We don't do a dist folder with pre-built javascript files and I don't have any plans on doing that.

The rest is probably fine to move. Even requirejs can be moved as well I think.

cheddar commented 10 years ago

Hrm, fwiw, the whole client directory with javascriptified code is also getting pushed to npm. I actually needed that in order to init the library in a node.js program I'm working on. So, please don't remove it without include some method of simple access to the data files (I do understand that if I want a different configuration of data files, I need to include that on my own).

I had just assumed that since you were pushing all of the pre-built js and the minified stuff, that the npm module was using that. But, now that I look at index.js, it's clear that it is loading coffee script.

jgable commented 10 years ago

I'll try to clean this up when I get a chance, hopefully this weekend. I'm going to leave this open in the mean time in case you want to update your PR.

cheddar commented 10 years ago

I'm good with cleaning up the PR, I'm just not exactly certain what you would like me to do. Is it just put coffeescript back into package.json and you are good?

jgable commented 10 years ago

Yeah, that's a good start. From there I can remove that dist folder for the npm package and tinker with the other dependencies to make sure they can be moved over to dev. I'll probably switch from cake to grunt as well.

cheddar commented 10 years ago

@jgable Ok, just put coffee back into dependencies. My OCD also got the better of me and now package.json is alphabetized!

cheddar commented 10 years ago

It looks like travis failed because it couldn't get an npm dependency (minimatch), oh the joys of npm. It should work on a restart, though, I think. I just cannot restart it :)

jgable commented 10 years ago

There is a couple problems with npm on node 0.8 right now, I have created an issue to track it npm/npm#5328.

jgable commented 10 years ago

I'll try to get my changes in for this this weekend and hope to publish it up as a new version then.

Thanks for taking the time to put this together and update it.