-
Notifications
You must be signed in to change notification settings - Fork 47
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
Refactor - Consolidating duplicate code. Minor bug fixes #73
base: master
Are you sure you want to change the base?
Conversation
var document = new YamlDocument(@"a: | ||
x: 1 | ||
b: 2 | ||
c: 3"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I moved the check for the lack of a root object from the call to ReplaceKey to the constructor since it could be checked with no other data required.
Because of this, the constructor now throws the Root Element Not Present error.
This test was modified to only validate the empty path exception, and a new test was added below to verify that the no root element exception is thrown by the constructor.
Looks like npm is throwing a few minor security warnings and that is breaking the build:
|
I started using magic-chunks recently and after looking through the code, I wanted to contribute a little.
I saw a decent amount of duplicated code blocks and some minor issues with grammar and parameter order that I wanted to resolve.
All tests pass with only two minor changes to the tests themselves. One of them was to eliminate the hiding of an error due to the order that the parameters were being checked. (see inline note).
Please review and provide feedback.
Thanks for making this tool, it's quite helpful!