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

Yaml switch #24

Merged
merged 5 commits into from Jun 1, 2012
Merged

Yaml switch #24

merged 5 commits into from Jun 1, 2012

Conversation

enyo
Copy link
Contributor

@enyo enyo commented May 7, 2012

Again, as requested, a separate pull request for the yaml parser switch.

I agree with you: it shouldn't break existing users. At the same time: The other yaml parser is just.. better. So if users switch to it, they shouldn't have any problems with their .yaml files.

The only thing they have to do, is to install the other yaml parser.

If you want I can check if there's a nice way to check if either of the yaml modules is installed, thus not braking backwards compatibility.

PS.: I based this pull request on the other one, since you said you wanted to pull it in anyway.

@enyo
Copy link
Contributor Author

enyo commented May 7, 2012

Ok.. So I added the Visionmedia YAML parser as fallback if the nodeca parser isn't available.

I haven't found a better way than to simply catch the require() exception.

@enyo
Copy link
Contributor Author

enyo commented May 20, 2012

Is there any reason you didn't pull this in yet? Are you not convinced that nodeca's yaml parser is better? Or is there something wrong with my request?

lorenwest added a commit that referenced this pull request Jun 1, 2012
@lorenwest lorenwest merged commit b946411 into node-config:v0.4.14 Jun 1, 2012
@lorenwest
Copy link
Collaborator

Apologies for taking so long to merge this. I'll get a new version published to npm shortly.

@enyo
Copy link
Contributor Author

enyo commented Jun 1, 2012

No problem! I have pending pull requests for weeks sometimes before I have the time to merge them. I just wanted to make sure that everything was OK.

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.

None yet

2 participants