cloudfoundry-community / node-cfenv

easy access to your Cloud Foundry application environment for node
Apache License 2.0
74 stars 20 forks source link

Best practice for leveraging cfEnv? #12

Open srl295 opened 8 years ago

srl295 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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

srl295 commented 8 years ago

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

srl295 commented 8 years ago

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