-
Notifications
You must be signed in to change notification settings - Fork 53
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 parsing empty yaml documents #228
Fix parsing empty yaml documents #228
Conversation
Signed-off-by: German Lashevich <[email protected]>
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.
LGTM!
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.
Do you mind adding a test to ensure that this feature is working?
Disclaimer: I'm not a golang developer. I was thinking about writing a test before implementing the fix, but I couldn't figure out how to do that nicely and without restructuring the existing code. So maybe I can get some advice here. This is the function which would be tested: func parseResources(paths []string, resourceFunc func([]byte) error) error It accepts a list of file paths or In a test, I can create a temporary file with content suitable for the test, pass the path to the function together with a callback function and check if This doesn't seem to be elegant but should work. Or maybe there are other ideas? |
👋 Hello, I wrote a quick test you can copy verbatim or not and use as inspiration to help move this issue along: New file
|
@neil-hickey awesome, thanks! I've added the test. It fails on the develop branch:
It passes on the feature branch:
|
Signed-off-by: German Lashevich <[email protected]>
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.
LGTM
Fixes #201