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

nsq_to_file: revert gzip behavior change #1120

Merged
merged 1 commit into from
Jan 7, 2019

Conversation

ploxiln
Copy link
Member

@ploxiln ploxiln commented Jan 6, 2019

In case we're all swayed by @jehiah's arguments in #1117 (comment) I opened this easy PR.

I'll squash before merge - I left the mechanical revert separate from additional tweaks needed due to the rest of the refactor, for possibly easier review.

cc @mreiferson

@jehiah
Copy link
Member

jehiah commented Jan 7, 2019

@mreiferson thoughts?

@jehiah jehiah requested a review from mreiferson January 7, 2019 01:00
Copy link
Member

@mreiferson mreiferson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks

This reverts commit 5ea1012.

also some further tweaks due to FileLogger refactor
@ploxiln ploxiln force-pushed the to_file_gzip_multi branch from 5df7725 to 6b4b720 Compare January 7, 2019 18:07
@mreiferson mreiferson merged commit 6d6f453 into nsqio:master Jan 7, 2019
@mreiferson
Copy link
Member

As a followup, I feel like we should comment the reason why we sync the way we do in that function? WDYT @ploxiln?

@ploxiln
Copy link
Member Author

ploxiln commented Jan 8, 2019

sure, will add in #1123

@ploxiln ploxiln deleted the to_file_gzip_multi branch February 2, 2019 06:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants