Skip to content
This repository has been archived by the owner on Aug 30, 2022. It is now read-only.

Use thrift 0.11.0 #94

Merged
merged 2 commits into from
May 2, 2018
Merged

Conversation

isaachier
Copy link
Contributor

Signed-off-by: Isaac Hier isaachier@gmail.com

src/jaegertracing/thrift-gen/zipkincore_constants.cpp
src/jaegertracing/thrift-gen/zipkincore_types.cpp
src/jaegertracing/utils/ErrorUtil.cpp
src/jaegertracing/utils/HexParsing.cpp
src/jaegertracing/utils/RateLimiter.cpp
src/jaegertracing/utils/Regex.cpp
Copy link
Member

@yurishkuro yurishkuro Apr 29, 2018

Choose a reason for hiding this comment

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

couldn't these list be automated? like find src -name '*cpp'

Copy link
Contributor Author

@isaachier isaachier Apr 29, 2018

Choose a reason for hiding this comment

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

Ya this is a huge inconvenience of CMake but the problem is config vs. build time. CMake generates a Makefile once upon configuration. If you run CMake, then add a new file, and try running make, the generated Makefile has no idea you did anything. CMake would be able to recursively search the directories, but make isn't going to do that for you. So you need to make a hard-coded list. Details here:

We do not recommend using GLOB to collect a list of source files from your source tree. If no CMakeLists.txt file changes when a source is added or removed then the generated build system cannot know when to ask CMake to regenerate.

https://cmake.org/cmake/help/v3.0/command/file.html

@codecov
Copy link

codecov bot commented Apr 29, 2018

Codecov Report

Merging #94 into master will increase coverage by 0.67%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #94      +/-   ##
==========================================
+ Coverage   89.05%   89.73%   +0.67%     
==========================================
  Files          96       96              
  Lines        2321     2318       -3     
==========================================
+ Hits         2067     2080      +13     
+ Misses        254      238      -16
Impacted Files Coverage Δ
src/jaegertracing/UDPTransport.h 100% <ø> (ø) ⬆️
src/jaegertracing/UDPTransport.cpp 95% <ø> (ø) ⬆️
src/jaegertracing/utils/UDPClient.h 82.6% <ø> (ø) ⬆️
src/jaegertracing/utils/UDPClient.cpp 100% <ø> (ø) ⬆️
src/jaegertracing/net/URI.cpp 100% <100%> (ø) ⬆️
src/jaegertracing/net/http/Header.h 100% <100%> (ø) ⬆️
src/jaegertracing/net/http/Request.cpp 100% <100%> (ø) ⬆️
src/jaegertracing/net/http/Response.cpp 90% <100%> (-0.25%) ⬇️
src/jaegertracing/testutils/MockAgent.cpp 81.51% <100%> (-0.16%) ⬇️
src/jaegertracing/Tracer.cpp 92.22% <0%> (+1.11%) ⬆️
... and 3 more

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 7dd252f...d4e8bb4. Read the comment docs.

@isaachier
Copy link
Contributor Author

Anyway, this sadly does not resolve jaegertracing/jaeger-idl#35, but I can use a patchfile to maintain the changes I used to fix it.

@isaachier
Copy link
Contributor Author

This should resolve #45.

Signed-off-by: Isaac Hier <ihier@uber.com>
Signed-off-by: Isaac Hier <ihier@uber.com>
@isaachier isaachier merged commit 2d556ab into jaegertracing:master May 2, 2018
@isaachier isaachier deleted the upgrade-thrift branch May 2, 2018 21:52
vadorovsky pushed a commit to vadorovsky/jaeger-client-cpp that referenced this pull request May 4, 2018
* Upgrade Thrift to 0.11.0

Signed-off-by: Isaac Hier <ihier@uber.com>

* Update copyrights on touched files

Signed-off-by: Isaac Hier <ihier@uber.com>
@ringerc
Copy link
Contributor

ringerc commented Aug 23, 2018

Using a patchfile is a good alternative.

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

Successfully merging this pull request may close these issues.

3 participants