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

Include the config-loader again #54

Closed
lholmquist opened this issue Feb 8, 2018 · 5 comments
Closed

Include the config-loader again #54

lholmquist opened this issue Feb 8, 2018 · 5 comments
Assignees

Comments

@lholmquist
Copy link
Member

lholmquist commented Feb 8, 2018

After talking with @lance, we decided it might be better to have the config loader integrated back into this library.

I think if no config is passed, then it just does the default stuff(calls config-loader), but if a config is passed in, then it will use that instead

I think i was trying to be to smart with this #5

@lholmquist
Copy link
Member Author

lholmquist commented Feb 8, 2018

This will change the method signature from func(config, settings) to just func(settings)

if you still want to pass in a custom config object, add it to settings.config, likewise, if you need to override something in the config loader, pass that in with settings.openshiftConfigLoader

also going to raise the node version needed to 8.x

Once this change is in, we will probably bump to 1.0.0 since it is a breaking change

@lholmquist
Copy link
Member Author

@tavogel wanted to give you a heads up

lholmquist added a commit to lholmquist/openshift-rest-client that referenced this issue Feb 9, 2018
* It is no longer necessary to include the openshift-config-loader separate.  By default, just calling the rest client will do the default config loading.
If a user needs to pass a config into the client, use the settings object.  ex:  settings.config = {...}

This is a breaking change from the previous version

fixes nodeshift#54
@tavogel
Copy link
Contributor

tavogel commented Feb 10, 2018

Thanks. This will be breaking as we don't use local configuration but as long as we can still override it should be ok

@lholmquist
Copy link
Member Author

@tavogel yea, you can override using settings.config = {...}

@lholmquist
Copy link
Member Author

@tavogel if you wanted to give the new version a try, i created a "canary" version as 0.12.0-0

@ghost ghost removed the needs-review label Feb 12, 2018
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

2 participants