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

Allow config.json location to be configured by argument to app.js #146

Merged
merged 1 commit into from
Mar 25, 2014

Conversation

rowanhill
Copy link
Contributor

Hi,

Following on from #137, this change allows the location (and name) of config.json to be configured via a command line argument to app.js. This allows I/O Docs to be run without any modifications within the repository.

The downside is that npm start can't be run with arguments (they don't get passed through to the specified script), until issue 3494 in npm is resolved. Instead, supervisor (or node) has to be run manually.

I know that's not ideal, but I think this is a big enough win to justify the pain (until the npm issue is resolved).

Thanks,
Rowan

@mtougeron
Copy link
Contributor

The code looks good but I'm curious why a simple symlink of the config.json won't work for this? It "feels" kind of hacky to me.

@rowanhill
Copy link
Contributor Author

My thinking was:
a) It feels bit cleaner to me to be able to run I/O Docs without having to put anything in the repository
b) Symlinks are a bit of a pain in Windows (this is actually the main motivator for me personally)
c) It allows easy switching between different configurations (which, admittedly, is quite rare, but it would help with the integration testing in #153)

mtougeron added a commit that referenced this pull request Mar 25, 2014
Allow config.json location to be configured by argument to app.js
@mtougeron mtougeron merged commit 0377b78 into mashery:master Mar 25, 2014
@rowanhill rowanhill deleted the configurable-config-json branch March 25, 2014 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants