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

Add amqp reporter support #130

Merged
merged 16 commits into from
Jun 18, 2019
Merged

Add amqp reporter support #130

merged 16 commits into from
Jun 18, 2019

Conversation

rustatian
Copy link
Contributor

@rustatian rustatian commented Jun 13, 2019

No description provided.

reporter/amqp/amqp.go Outdated Show resolved Hide resolved
reporter/amqp/amqp_test.go Outdated Show resolved Hide resolved
@coveralls
Copy link

coveralls commented Jun 13, 2019

Coverage Status

Coverage increased (+11.5%) to 73.999% when pulling e2f0d1b on ValeryPiashchynski:add_amqp_reporter_support into b42a292 on openzipkin:master.

@rustatian
Copy link
Contributor Author

rustatian commented Jun 13, 2019

@basvanbeek @jcchavezs I have fixed all tests issues except appveyor (I've never worked with this CI). Maybe somebody can help me with that? I've added rabbit target to the appveyor.yaml, but it doesn't work

@basvanbeek
Copy link
Member

maybe this AppVeyor install script for rabbitmq helps?

https://gist.github.com/FeodorFitsner/06b377df42ba8f398909

@basvanbeek
Copy link
Member

@ValeryPiashchynski if wanting to throw the towel in the ring I am ok with disabling the windows build on the AMQP unit test for now.

@rustatian
Copy link
Contributor Author

rustatian commented Jun 14, 2019

@ValeryPiashchynski if wanting to throw the towel in the ring I am ok with disabling the windows build on the AMQP unit test for now.

It would be great. Because I am really don't know how to make this PR working with appveyor.

@basvanbeek
Copy link
Member

Hi Valery... just proceed with adding that build tag. Once appveyor gives the green light again I will merge.

@rustatian
Copy link
Contributor Author

Hi Valery... just proceed with adding that build tag. Once appveyor gives the green light again I will merge.

Hi @basvanbeek. Do you mean, that I should trigger the same build until appveyor gives the green light?

@basvanbeek
Copy link
Member

@ValeryPiashchynski sorry for the confusion... I just pushed a commit to add the build tag. it should now end with all greens :)

@jcchavezs jcchavezs merged commit a035034 into openzipkin:master Jun 18, 2019
@jcchavezs
Copy link
Contributor

I should have merge this PR as squash commit. My bad. Sorry guys.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants