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
Does not close stdout when running DlmeJsonResourceWriter. Closes #54. #55
Conversation
lib/dlme_json_resource_writer.rb
Outdated
@@ -21,6 +21,10 @@ def serialize(context) | |||
"#{JSON.pretty_generate(adjusted)}" | |||
end | |||
|
|||
def close | |||
@output_file.close unless (@output_file.nil? || @output_file.tty? || @output_file == $stdout) |
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.
Can you explain why we need to check for tty?
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.
or nil for that matter?
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.
Should this patch be made upstream? https://github.com/traject/traject/blob/master/lib/traject/line_writer.rb#L60
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.
Can you refactor to
def close
return if @output_file == $stdout
super
end
0cfb665
to
4233789
Compare
Upstream PR: traject/traject#202 |
4233789
to
42eed4b
Compare
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.
Can you please add a comment referencing that this customization is needed until traject/traject#202 is resolved?
42eed4b
to
7c06806
Compare
7c06806
to
cd6ca4f
Compare
No description provided.