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

Dashboard example includes confusing code #13

Open
ericallam opened this issue Apr 28, 2012 · 2 comments
Open

Dashboard example includes confusing code #13

ericallam opened this issue Apr 28, 2012 · 2 comments

Comments

@ericallam
Copy link
Contributor

The code itself isn't confusing, I'm just confused as to why this stuff is in the client app:

logging_gateway = engine.logging_gateway.create();
stdout_client = engine.logging_stdout_client.create();
logging_gateway.add_logger(stdout_client);

logging_opts = {
    logging_gateway: logging_gateway
};

intake = engine.intake.create(logging_opts);
exhaust = engine.exhaust.create(logging_opts);
cylinder = engine.cylinder.create(logging_opts);

It doesn't seem like it's necessary, or am I missing something? Can it be used for communication with the piston server?

@rehanift
Copy link
Owner

That code is what starts the "server" aspect of engine.js: an intake, an exhaust, and one or more cylinders (in fact, this is exactly whats in script/server.js). Its included in the app.js file because I didn't want the user to have to start a server, then start the express app. I do see how this can be confusing, though.

It should be possible to remove this code and start the server with script/server.js, then start the express application separately. Does that seem better to you?

On a related note, I have toyed with the idea of bundling this "bootstrap" code into a convenience method, something like engine.server.create() but I haven't gotten around to it yet. Another idea I have been kicking around in my head is creating a command line tool that will start the server for you and expose some config options, ie. engine server start --cylinders=3

@ericallam
Copy link
Contributor Author

Ah ok I didn't realize you could run both the server and the client in the same node process, that makes sense. I think just some better documentation in the app.js file would do the trick.

I do like the idea of creating a "server factory" and along with that an easy to start server with options, but I would go the npm scripts route (something like npm start which would start the server).

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

No branches or pull requests

2 participants