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

Revert v2 zipkin support due to #7415. #7773

Merged
merged 1 commit into from May 20, 2019

Conversation

Projects
None yet
2 participants
@stuhood
Copy link
Member

commented May 20, 2019

Revert "Collect Zipkin spans for v2 engine (#7342)"

This reverts commit 99e6e3c.


As described in #7415, background workunits fail when zipkin is enabled.

@stuhood stuhood added this to the 1.16.x milestone May 20, 2019

@stuhood stuhood requested review from cattibrie, illicitonion, cosmicexplorer and wisechengyi and removed request for cosmicexplorer May 20, 2019

@jsirois

This comment has been minimized.

Copy link
Member

commented May 20, 2019

Was this a git revert ...? If so, I'd much rather see the raw git revert commit message than Problem / Solution, etc. It highlights in history a mechanical revert as different as a revert to potentially inspect further for human error.

@stuhood

This comment has been minimized.

Copy link
Member Author

commented May 20, 2019

Was this a git revert ...? If so, I'd much rather see the raw git revert commit message than Problem / Solution, etc. It highlights in history a mechanical revert as different as a revert to potentially inspect further for human error.

Yea, it was, but there was a merge conflict, and I forgot to commit without -m.

@stuhood

This comment has been minimized.

Copy link
Member Author

commented May 20, 2019

Having said that, if I used only the bare revert message, the context for why it was reverted would be lost.

@jsirois

This comment has been minimized.

Copy link
Member

commented May 20, 2019

How about including it's message? IE:

Revert "Ignore `RUSTSEC-2019-0003`. (#7766)"

This reverts commit 91a88ba0d3fe8731717cd8d64424dc2b90952162.

[more verbage]
@stuhood

This comment has been minimized.

Copy link
Member Author

commented May 20, 2019

@jsirois : How does that look?

@stuhood stuhood merged commit 58706c1 into pantsbuild:master May 20, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@stuhood stuhood deleted the twitter:stuhood/revert-zipkin-v2 branch May 20, 2019

stuhood added a commit that referenced this pull request May 20, 2019

Revert v2 zipkin support due to #7415. (#7773)
### Problem

As described in #7415, background workunits fail when zipkin is enabled. 

### Solution

Revert #7342 until that can be fixed.

cattibrie added a commit to cattibrie/pants that referenced this pull request May 29, 2019

cattibrie added a commit to cattibrie/pants that referenced this pull request May 31, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.