Skip to content

nsq_tail multi-topic and build stuff #960

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

Closed
wants to merge 6 commits into from
Closed

Conversation

ploxiln
Copy link
Member

@ploxiln ploxiln commented Nov 5, 2017

I just squashed and re-titled the commits from #957

soar added 5 commits November 5, 2017 12:52
with optional flag "--print-topic" to print topic name before message body
avoid "use of internal package not allowed" error in this case
by forcing the import path to be as if it were the upstream repo
now works for any $GOPATH, not just $HOME/gopath
build in a golang-based container, then copy
resulting binaries into a minimal alpine-based container
@ploxiln
Copy link
Member Author

ploxiln commented Nov 5, 2017

@mreiferson I could also squash all the build-related commits together if you like

use pointers to nsq.Consumer, rather than copies of the structs
otherwise they don't fully stop
@ploxiln
Copy link
Member Author

ploxiln commented Nov 5, 2017

I also added a tweak to the stopping, see the last commit @soar

@mreiferson
Copy link
Member

@ploxiln LGTM, you want to leave that last commit separate?

@mreiferson
Copy link
Member

also, I think this is another case where you could push to the original PR's branch instead?

@soar
Copy link
Contributor

soar commented Nov 5, 2017

How this PR got increased coverage with same changes as mine? :-D

@ploxiln
Copy link
Member Author

ploxiln commented Nov 5, 2017

the coverage fluctuates randomly, one of these days I'll get around to figuring out why :)

@ploxiln
Copy link
Member Author

ploxiln commented Nov 5, 2017

I'll squash my fix in. I didn't think of the fact that I might be able to push to the original branch because I'm an admin member, I'll try it ...

@ploxiln
Copy link
Member Author

ploxiln commented Nov 5, 2017

I was able to force-push this to #957

@ploxiln ploxiln closed this Nov 5, 2017
@ploxiln ploxiln deleted the multitail branch April 30, 2018 03:48
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