search

kimlai / melikeyradio

Simple webradio/musicblog engine.
1 stars 1 forks source link

Merging AmdModuleCompiler #13

Closed henrycatalinismith closed 10 years ago

henrycatalinismith commented 10 years ago

I really like that AmdModuleCompiler class. How would you feel about us including that code in our Twig.js fork?

kimlai commented 10 years ago

Yes sure. I'm not completely happy with it though, making it extend a The ModuleCompiler as it is feels hackish. I was thinking about making the methods compileClassHeader and compileClassFooter abstract, and then having AmdModuleCompiler and GoogModuleCompiler extend ModuleCompiler.

I've only been toying around with both requirejs and twigjs so maybe you'd have some insight about this. I might try it myself if I find the time.

kimlai commented 10 years ago

Oh cool. Any chance you'd try to merge it upstream ?

henrycatalinismith commented 10 years ago

Oops, sorry I forgot to answer this!

To be honest, I think @schmittjoh has a whole bunch of other projects that are keeping him pretty busy these days. I don't want to pile more pull requests onto him. I definitely think this would be a valuable addition to the main Twig.js project, but until that happens I'm happy to look after it in the BaseKit fork.

schmittjoh commented 10 years ago

Since you already contributed several patches to twig.js and seem to know the code base quite well, what do you think if I give you commit access so you can make your changes without going through pull requests?

henrycatalinismith commented 10 years ago

I think I could cope with that. We're pretty invested in twig.js at @basekit, which means I already have to tread quite carefully in the fork, or I risk breaking the product. It'd be really awesome to get our work merged back into the main repository where everyone else can benefit from it :)

schmittjoh commented 10 years ago

Great :)

I've added you as a collaborator to the repository.

On Sun, Mar 2, 2014 at 10:09 PM, Henry Smith notifications@github.comwrote:

I think I could cope with that. We're pretty invested in twig.js at @basekit https://github.com/basekit, which means I already have to tread quite carefully in the fork, or I risk breaking the product. It'd be really awesome to get our work merged back into the main repository where everyone else can benefit from it :)

Reply to this email directly or view it on GitHubhttps://github.com/kimlai/melikeyradio/issues/13#issuecomment-36466964 .