uber / tchannel-go

Go implementation of a multiplexing and framing protocol for RPC calls
http://uber.github.io/tchannel/
MIT License
490 stars 84 forks source link

Vendor apache-thrift due to breaking changes with version >= 0.10.0 #875

Closed gandhikrishna closed 2 years ago

gandhikrishna commented 2 years ago

Apache thrift versions >=0.10.0 contain breaking changes this is primarily due to the introduction of ctx parameter and the difference in generated goTypes. Today tchannel-go pins to <0.10.0. This conflict causes a lot of pain for projects building with any projects that have Apache thrift dependencies with higher versions.

This PR aims to vendor Apache Thrift inside tchannel go.

CLAassistant commented 2 years ago

CLA assistant check
All committers have signed the CLA.

codecov[bot] commented 2 years ago

Codecov Report

Merging #875 (5f87656) into dev (ce5b440) will increase coverage by 0.38%. The diff coverage is n/a.

:exclamation: Current head 5f87656 differs from pull request most recent head 62d2364. Consider uploading reports for the commit 62d2364 to get more accurate results

@@            Coverage Diff             @@
##              dev     #875      +/-   ##
==========================================
+ Coverage   88.75%   89.14%   +0.38%     
==========================================
  Files          43       43              
  Lines        4439     4439              
==========================================
+ Hits         3940     3957      +17     
+ Misses        379      365      -14     
+ Partials      120      117       -3     
Impacted Files Coverage Δ
outbound.go 81.87% <0.00%> (-6.44%) :arrow_down:
inbound.go 82.38% <0.00%> (+1.55%) :arrow_up:
relay.go 87.82% <0.00%> (+2.13%) :arrow_up:
connection.go 89.37% <0.00%> (+3.46%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update ce5b440...62d2364. Read the comment docs.

cinchurge commented 2 years ago

the long import name feels a little verbose, but not a big deal. lgtm