search

maccman / sprockets-commonjs

Adds CommonJS support to Sprockets
178 stars 66 forks source link

Some cleanup #8

Closed jamesarosen closed 12 years ago

jamesarosen commented 12 years ago
  1. move sprockets/engine to sprockets/commonjs/engine so it doesn't conflict with any other Sprockets Engines
  2. add Sprockets::CommonJS.default_namespace= so clients don't have to monkey-patch to change it
  3. make Sprockets::CommonJS#prepare and #evaluate protected, like they are for Tilt::Template (the parent class)
  4. make helper method Sprockets::CommonJS#namespace private
  5. extract helper method Sprockets::CommonJS#common_js_module?
  6. extract the CommonJS module definition JavaScript into a constant
maccman commented 12 years ago

Awesome - few nitpicks, but am keen to get this merged.

Also, just want to make sure you're aware, but there's an edge branch for the latest version of Sprockets: https://github.com/maccman/sprockets-commonjs/tree/edge

jamesarosen commented 12 years ago

I'm happy to do this against the edge branch as well.

maccman commented 12 years ago

That would be awesome.