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

Best practice for leveraging cfEnv? #12

Open
srl295 opened this issue Nov 18, 2015 · 4 comments
Open

Best practice for leveraging cfEnv? #12

srl295 opened this issue Nov 18, 2015 · 4 comments

Comments

@srl295
Copy link

srl295 commented Nov 18, 2015

Say I am writing an SDK (just suppose…)

Is the following reasonable, and if so, should it be documented as a best practice?

OLD:

 var appEnv = require('cfenv').getAppEnv();
 var someClient = require('Some Random Thing').setup({ credentials: appEnv.getServiceCreds(/Something.*/) });

IMPROVED?

 var appEnv = require('cfenv').getAppEnv();
 var someClient = require('Some Random Thing').setup({ appEnv: appEnv });

implemented within the SDK by:

 … function setup(opts) { 
    if(opts.appEnv && !opts.credentials) {  // if appEnv is available, use it
     opts.credentials = opts.appEnv.getServiceCreds( someRegex );
    }

Any pitfalls? My idea is to reduce the amount of copy and paste user code. User can just pass in appEnv if they have one, otherwise credentials can be supplied.

@pmuellr
Copy link
Member

pmuellr commented Nov 18, 2015

Ya, that seems reasonable. I'd document your "Some Random Thing" package as providing a setup() method that takes an options object, where one key can be appEnv and is expected to be the result of a cfenv.getAppEnv() call.

I'm not sure about documenting it as best practice, since yours is the first suggestion I've seen of someone wanting to do this. Let us know how it works out! Maybe it WILL BE a best practice!

@srl295
Copy link
Author

srl295 commented Nov 18, 2015

@pmuellr thanks. We'll see how it goes, I'll link back here. I'm also exporting the regex itself from the package.

@srl295
Copy link
Author

srl295 commented Nov 21, 2015

As promised, see https://www.npmjs.com/package/g11n-pipeline

@srl295
Copy link
Author

srl295 commented Dec 23, 2015

All OK, except for the issue #3 i hit.

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