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

Add Ability to pass SourceJS CLI options via object #243

Open
Alanoll opened this issue Dec 20, 2016 · 5 comments
Open

Add Ability to pass SourceJS CLI options via object #243

Alanoll opened this issue Dec 20, 2016 · 5 comments

Comments

@Alanoll
Copy link

Alanoll commented Dec 20, 2016

#241 added the ability to use NodeJS's require to load SourceJS as part of a NodeJS script.

This issue will need to add the ability to pass in the SourceJS CLI options via plain object to the startServer method.

It should follow the same inheritance/override pattern as the current CLI ability.

  1. Load Options via Options file
  2. Load Options via CLI
  3. Load Options via Options object
  4. Override using process.env
@Alanoll
Copy link
Author

Alanoll commented Dec 20, 2016

I'm thinking something simple matching the SourceJS options file:

{ hostname: '', port: '' }

Use of commander should be inspected throughout SourceJS, to ensure passed Options are used where neccesary (Tests come to mind).

A lot of SourceJS init occurs prior to the startServer method currently. Some of this logic may need to be moved so Options are respected properly.

@Alanoll
Copy link
Author

Alanoll commented Dec 20, 2016

I don't think this issue should hold up the release of 0.6.0. I'm not sure of the full scope of changes, and they may be large enough that pushing to the next release makes sense.

@robhrt7
Copy link
Member

robhrt7 commented Dec 28, 2016

Indeed, 0.6.0 just needs to be released already. But feel free to draft a PR whenever you're ready.

@Alanoll
Copy link
Author

Alanoll commented Jan 13, 2017

Just adding a note here, so I don't forget when I come back to this.

I think we should do what Norbert did in this changeset where he moved all the server logic into a server.js file. I can either wait for his PR to be accepted (post-0.6.0) or we just deal with the annoying merge.

@ndelangen
Copy link
Member

We could cherry-pick that change into a new branch and submit a new PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants