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

fix #130 with proper syntax & on master branch #233

Closed
wants to merge 3 commits into from

Conversation

maxleiko
Copy link

Hi there,

I've fixed #130 as you did in this commit but using Object.defineProperty instead of __defineGetter__.
Also, you've fixed it in branch gh and did not publish any nconf version on the npm registry with this patch. Do you plan to merge this PR and publish nconf in a near future?

@maxleiko
Copy link
Author

@indexzero any chances?

@indexzero
Copy link
Owner

Thanks for the contribution!

There are quite a bit of changes here. Would be happy to accept the PR for just the browserify support if you'd like to decouple them.

@indexzero indexzero closed this Aug 9, 2017
@maxleiko
Copy link
Author

maxleiko commented Aug 10, 2017

@indexzero, in the end I've started my own config module tiny-conf as I needed the pluggable architecture in order to minimize bundled size & browser compat.

To be fair, my default storage is in-mem and is pretty much a copy/paste of your Memory store 👍

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

Successfully merging this pull request may close these issues.

Support using nconf in a (browserify-based) browser context
2 participants