Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

runtime cleanup #117

Closed
wants to merge 2 commits into from
Closed

runtime cleanup #117

wants to merge 2 commits into from

Conversation

b-laporte
Copy link
Member

This PR removes some runtime methods that were added for the playground application and that are now obsolete. It also pushes the transpiler in the compiler, as it doesn't make sense to keep them separate - especially if the playground shows samples with the transpiler (playground doc has also been updated accordingly btw.)

@benouat
Copy link
Member

benouat commented Apr 3, 2014

I kinda liked to have the transpiler separated.... 😕
It was making things clearer to me

@@ -1,6 +1,7 @@
var parser = require("./parser/index");
var treebuilder = require("./treebuilder/index");
var jsgenerator = require("./jsgenerator/index");
var transpiler = require("../transpiler");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Build is failing here (see https://travis-ci.org/ariatemplates/hashspace/builds/22138228):

Running "atpackager:runtime" (atpackager) task
>> hsp/compiler/compiler.js depends on ../transpiler which cannot be found.
Warning: There were 1 error(s) while packaging.� Use --force to continue.

Should be replaced by var transpiler = require("../transpiler/index");

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then we need to also include uglify-js in the noder build... I am working on this...

@b-laporte
Copy link
Member Author

Well, we all agree that we should introduce compilation options (e.g. to say if we want compile in 'commonJS' or 'globalVariable' mode) - so one of the option should be done for the desactivate transpiler. Btw. I added the transpiler to the compiler as I want to keep things simple - and having to call a compiler is already complex enough, so I would prefer to keep only one compilation step (but of course if some want to use more fine-tuned build scripts they should be allowed to...)
PS: I just pushed the dependency update - but I am not expert enough in atpackager to solve the missing uglify dependency (@divdavem thx for your help!)

@b-laporte b-laporte closed this in d2b222d Apr 15, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants