Support building with local dependencies (no Hunter) #46

ringerc · 2018-01-28T12:51:24Z

jaeger-cpp defaults to using Hunter to download its dependencies off the 'net,
even if they exist locally. This is convenient for development but not
practical for some production environments. It also makes life hard for clients
that want to link to jaeger-cpp since Hunter doesn't install those
dependencies. It's necessary to also use Hunter in apps that use a jaeger-cpp
built this way... and that's not always practical.

Accordingly, add support for finding jaeger-cpp's dependencies via the normal
CMake package discovery mechanism.

A sligtly unsightly hack is required for nlohmann json, because its header
moved from json.hpp to nlohmann/json.hpp in 2.1.0.

This introduces support for building with

cmake -DHUNTER_ENABLED=0

WIP:

nothing packages nlohmann json 2.1.0 yet, 2.0.2 is widespread.
but jaegertracing's code doesn't appear to support 2.0.2. For now you
should work around it by installing 2.1.x locally from source.
(no bug opened yet)
will fail to compile tests if the local thrift is 0.9.1 since there are
committed generated files from 0.9.2 (Thrift IDL not in main repository, no build support #45) and there's no mechanism
to regenerate them.

The nlohmann json issue is beyond my C++-fu, but it's easy enough to install 2.1.x and it's just a header. A pain, but manageable.

The Thrift issue #45 is a bit more of an issue; I'm on Fedora 25 with 0.9.1 right now, but Fedora Extras for RHEL7 also carries 0.9.1. More to the point, it's probably bad to rely on a code generator's output being compatible across multiple versions of its library.

ringerc · 2018-01-28T12:52:59Z

Hrm, would also have to add support for non-hunter Travis builds before mergeable.

isaachier

Good work. I will wait to approve until you get build to work. Also, we can potentially add a Travis CI build with no Hunter to see how this works.

isaachier · 2018-01-28T13:07:55Z

CMakeLists.txt

@@ -59,12 +59,12 @@ list(APPEND package_deps thrift)
 hunter_add_package(opentracing-cpp)
 # Not `${hunter_config}` because OpenTracing provides its own
 # OpenTracingConfig.cmake file
-find_package(OpenTracing CONFIG REQUIRED)
-list(APPEND LIBS OpenTracing::opentracing-static)
+find_package(OpenTracing ${hunter_config} REQUIRED)


isaachier · 2018-01-28T13:11:15Z

CMakeLists.txt

@@ -283,7 +287,7 @@ if(BUILD_TESTING)
      GTEST_HAS_TR1_TUPLE=0
      GTEST_USE_OWN_TR1_TUPLE=0)
  target_link_libraries(
-      UnitTest testutils GTest::main jaegertracing-static ${LIBS})
+      UnitTest testutils ${GTEST_MAIN_LIBRARY} jaegertracing-static ${LIBS})


Not sure if this is compatible with Hunter. May need to do something like this:

if(HUNTER_ENABLED) set(gtest_lib GTest::main) else() set(gtest_lib ${GTEST_MAIN_LIBRARY}) endif()

isaachier · 2018-01-28T13:12:27Z

src/jaegertracing/baggage/RemoteRestrictionJSON.h

+  #include <json.hpp>
+#else
+  #include <nlohmann/json.hpp>
+#endif


Interesting. Another possibility is including the header somewhere because I think it is a header-only library.

Removed from latest push since cpp-client doesn't build against older headers due to code compat issues anyway (#47)

isaachier · 2018-01-28T13:12:39Z

src/jaegertracing/testutils/CMakeLists.txt

+  target_link_libraries(testutils thrift::thrift_static)
+else()
+  target_link_libraries(testutils ${THRIFT_LIBRARIES})
+endif()


ringerc · 2018-01-28T13:19:45Z

No major changes in this push, no need to re-examine.

Will try to address the Thrift stuff but am rapidly running out of time for what was going to be a 1-day prototype. I can't even link it to postgres yet :( so may have to abandon and grab Zipkin or something for now.

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

ringerc · 2018-01-29T01:23:58Z

Also consider that opentracing-cpp does not its self use Hunter, so it's a requirement introduced by jaeger cpp-client.

ringerc · 2018-01-31T06:17:31Z

Pushed new commits. In particular I've now added the docs on top, since they didn't make much sense without local dependencies support.

I don't think I can get Travis support in too simply because of the quirky dependencies on Thrift and json-cpp. But anyway, it works...

ringerc · 2018-02-01T04:06:53Z

I landed up mostly rewriting the build system to make this work fairly cleanly. It now follows https://rix0r.nl/blog/2015/08/13/cmake-guide/ practices: modular CMakeLists.txt, uses IMPORT targets, etc. You'll find it a lot easier to follow and work with, and it doesn't have to special case stuff for Hunter anymore.

ringerc · 2018-02-01T04:08:54Z

I also got rid of the testutils subproject, instead building them as part of the test executables.

I haven't looked closely at how to do a local-dependencies build in Travis yet. The usual technique would be listing package dependencies but the very specific Thrift dependency and very new json dependency make that impractical.

ringerc · 2018-02-01T05:25:22Z

Hrm, the unit tests fail for some reason...

ringerc · 2018-02-01T05:56:39Z

Test failures were due to finding the installed headers. Fixed.

codecov · 2018-02-01T06:16:57Z

Codecov Report

Merging #46 into master will decrease coverage by 0.04%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master      #46      +/-   ##
==========================================
- Coverage   88.51%   88.46%   -0.05%     
==========================================
  Files          93       93              
  Lines        2246     2246              
==========================================
- Hits         1988     1987       -1     
- Misses        258      259       +1

Impacted Files	Coverage Δ
src/jaegertracing/UDPTransport.cpp	`91.83% <0%> (-2.05%)`	⬇️
src/jaegertracing/reporters/RemoteReporter.cpp	`73.33% <0%> (ø)`	⬆️

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 8d49317...2022642. Read the comment docs.

ringerc · 2018-02-01T07:58:21Z

@isaachier OK, think we're good to go :)

I fixed the coverage issue.

isaachier · 2018-02-01T14:07:22Z

CMakeLists.txt

-else()
-  list(APPEND LIBS ${THRIFT_LIBRARIES})
+find_package(thrift 0.9.2 ${hunter_config} REQUIRED)
+if(thrift_VERSION VERSION_GREATER "0.9.3")


Maybe add OR VERSION_LESS "0.9.2".

That's handled by find_package. It doesn't have good support for saying "these two versions ONLY" hence the warning.

Right. I guess my point is that find_package works differently with Hunter vs. non-Hunter.

Here's a brief overview. CMake offers two ways of finding packages (and maybe a third but IDK about it): modules and configs. Modules refer to the Find<Package> modules that you added here and that many people use in CMake builds. The problem is inherent redundancy of many projects. Why does everyone need to add these modules? So CMake offered another approach whereby the package declares its location during its CMake build, then other packages can easily locate it. The latter method is called the config approach.

Hunter relies SOLELY on the config approach. That means none of your Find<Package> files will be called at all. Relying on any code in there isn't going to work.

Yeah, I realise that. That's why ${hunter_config} substitutes the CONFIG option to find_package.

Have a look at the CMake package config files CMake generates from the Hunter-built packages. You'll see that they add_library(blah SHARED IMPORT) and setting INTERFACE properties like INTERFACE_INCLUDE_DIRECTORIES and INTERFACE_INCLUDE_DIRECTORIES on the resulting target. That's exactly what I'm doing in the Find packages too, to be consistent with CMake CONFIG mode. Look at the bundled CMake modules in /usr/share/cmake/Modules (or wherever yours are installed) e.g. FindGTest.cmake. You'll find that they also declare import targets and set properties on them for the library name, header paths, etc.

So it's not really to do with Hunter or non-Hunter. There are two things at play here:

Is it using CONFIG mode (as Hunter requests), MODULE mode where it only uses Find packages, or the default where it tries a module first then falls back to config-mode?

Does the resulting configuration created by a CONFIG file load or a Find module execution generate import-targets (new-style CMake), or just set variables (old-style CMake)?

What I've done is written the Find modules to produce the same import targets, etc, as what you get from loading the CONFIG module. So it all "Just Works". It's done how CMake's own bundled modules do it.

None of this is helped by the fairly erratic CMake documentation, but have a look at:

https://cmake.org/cmake/help/v3.3/command/add_library.html#imported-libraries

https://cmake.org/cmake/help/v3.3/prop_tgt/IMPORTED.html#prop_tgt:IMPORTED

https://cmake.org/cmake/help/v3.3/manual/cmake-packages.7.html

https://cmake.org/cmake/help/v3.0/prop_tgt/INTERFACE_INCLUDE_DIRECTORIES.html

https://cmake.org/cmake/help/v3.0/prop_tgt/INTERFACE_LINK_LIBRARIES.html

https://cmake.org/cmake/help/v3.0/prop_tgt/INTERFACE_COMPILE_DEFINITIONS.html

CMake takes care of the dependency chains, adding link libraries and include paths when you reference an import target, etc. It uses the properties set on the target for this. There's no need for a LIBS variable or any of that anymore.

This is also why the build-tree passed to headers in target_include_directories are PRIVATE and the install-path headers are INTERFACE. This tells CMake not to include the build-tree headers in the CMake config file it generates. And to add the install-target headers in the config file, but not use them when compiling the package its self, since it won't be installed yet.

CMake really needs a docs section that properly explains PRIVATE, PUBLIC and INTERFACE, explains IMPORT targets and how they're used in Find modules, etc. It's there, you just have to find all the scattered pieces from the docs and work out the whole picture from it. Dealing with this has been a massive pain, but hopefully it's nearly done.

isaachier

OK I see the changes and how they worked for you. The problem is merging this will break the Hunter build, which is currently the default. I'll try tweaking this to make it compatible with both. Good work and thanks for more docs!

isaachier · 2018-02-01T14:08:02Z

CMakeLists.txt

-project(jaegertracing VERSION 0.3.0)
+project(jaegertracing VERSION 0.3.0 LANGUAGES CXX)
+
+include(GNUInstallDirs)


Is this used?

No, good catch.

isaachier · 2018-02-01T14:08:36Z

CMakeLists.txt

 if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU" AND
    CMAKE_CXX_COMPILER_VERSION VERSION_LESS "4.9")
  set(boost_components regex)
  hunter_add_package(Boost COMPONENTS regex)
-  list(APPEND LIBS Boost::regex)


Does this break the Hunter build?

No, if there's a library to link assocated with Boost::regex (for old compilers where it's not header-only, apparently) it's specified using target_link_libraries for jaegertracing.

Though come to think of it, that should be unconditional. On newer g++ the Boost::regex target will be an INTERFACE library instead.

I'm not 100% sure why the build uses hunter_add_package(Boost COMPONENTS regex) for older gcc, and simply hunter_add_package(Boost) for newer. That seems ... weird. Wouldn't you just always ask for Boost::regex with COMPONENTS regex ?

So the reason for that is the header-only vs. library dependencies in Boost. Hunter needs to know which libraries it is going to build. I'd rather not build or use the regex library from Boost if standard C++11 regex is available.

isaachier · 2018-02-01T14:10:42Z

CMakeLists.txt

-  ARCHIVE DESTINATION "lib"
-  RUNTIME DESTINATION "bin"
-  INCLUDES DESTINATION "${include_install_dir}"
-)


Why remove this?

Move, not remove. Installation of the library target is in the library's own CMakeLists.txt in src/jaegertracing.

isaachier · 2018-02-01T14:11:16Z

CMakeLists.txt

@@ -371,3 +129,12 @@ install(
  NAMESPACE "${namespace}"
  DESTINATION "${config_install_dir}"
 )
+
+option(BUILD_TESTING "Build unit test" ON)


This option is built-in to CMake. No need to redeclare.

Thanks. My last significant CMake experience was a long time ago.

No worries :). I see this in many projects and always wonder why they don't use the built-in variable.

isaachier · 2018-02-01T14:11:46Z

CMakeLists.txt

+option(BUILD_TESTING "Build unit test" ON)
+if (BUILD_TESTING)
+  enable_testing()
+  add_custom_target(check COMMAND echo "run make test instead")


isaachier · 2018-02-01T14:12:29Z

README.md

+    git clone https://github.com/jaegertracing/cpp-client.git jaeger-cpp-client
+    mkdir jaeger-cpp-build
+    cd jaeger-cpp-build
+    cmake -DCMAKE_INSTALL_PREFIX=$HOME/jaeger-cpp-client ../jaeger-cpp-client


This command should still work with existing install directives.

I don't understand

I just mean this seems to imply the user must specify a prefix. It is optional, not a must.

isaachier · 2018-02-01T14:13:04Z

README.md

+
+    void teardownTracing()
+    {
+        opentracing::Tracer::Global()->Close


Does it need to be closed?

AFAICS if you don't close the tracer you might not flush all spans. But I guess it depends on the implementation details I haven't been able to dig into yet about whether any caching and delays are present for sending span closes.

I'd argue it's good practice; if you don't have to now, you probably will later when it starts buffering spans and sending them in chunks to reduce syscall overheads, etc.

It seems you are correct:

// Close is called when a tracer is finished processing spans. It is not // required to be called and its effect is unspecified. For example, an // implementation might use this function to flush buffered spans to its // recording system and failing to call it could result in some spans being // dropped.

https://github.com/opentracing/opentracing-cpp/blob/master/include/opentracing/tracer.h#L147

isaachier · 2018-02-01T14:13:37Z

README.md

+
+It's possible to use `jaeger-cpp` from C with appropriate `extern "C"` thunks
+and care around resource lifetime management etc. A real world example is the
+support for opentracing and jaeger in ngnix; see


Typo: nginx

isaachier · 2018-02-01T14:15:27Z

src/jaegertracing/CMakeLists.txt

+# Build the jaegertracing library its self
+cmake_minimum_required(VERSION 3.3)
+
+project(libjaegertracing VERSION ${jaegertracing_VERSION} LANGUAGES CXX)


This seems like a bad idea. It should all be one project. Not sure why splitting at all TBH.

Separate project: convenience and seems to be standard practice with cmake, but I don't feel strongly about it.

Splitting at all: readability, maintainability, separation of concerns. The tests don't intermix with the main library and trample on its settings, etc.

See also https://rix0r.nl/blog/2015/08/13/cmake-guide/ and many others.

If I land up adopting this seriously I'll probably send a followup pull to extract the test sources into a separate subtree and ensure there's zero inclusion of test sources by non-test code, really make the boundary clear. I'll also submit another to pull the thrift generated code out of the source tree and build it on demand, something that'll be much easier with the possible fix to #45 I've been looking at.

I have seen splitting the CMakeLists.txt file into multiple subdirectories, but I guess not subprojects. I see it is allowed in the link you pointed out, just hope it doesn't mess with other dependencies, etc.

OK, I'll remove the subproject

ringerc · 2018-02-01T14:54:00Z

@isaachier It passes Hunter builds in CI and on my local machine. I spent a fair while preserving that and making everything uniform.

The CI tests listed as passing on this pull used Hunter, not local builds.

ringerc · 2018-02-07T08:13:48Z

@isaachier Any thoughts?

isaachier · 2018-02-12T19:46:40Z

Sorry for delay here. I'd like to try this along with the changes in #48. Can you copy the modified Travis files there to allow for testing with and without Hunter?

Restructure the build system to follow CMake conventions: use import libraries consistently, separate CMakeLists.txt for targets, etc. Add Find packages to discover local dependencies not already supported by CMake natively. These are used when Hunter is disabled to allow jaeger cpp-client to use locally installed libraries. This is a preliminary step in splitting the test sources out of the library. Improves jaegertracing#38 Offline builds This introduces support for building with cmake -DHUNTER_ENABLED=0 Limitations for local dependency builds: - Requires a locally installed Thrift 0.9.2 or 0.9.3 EXACTLY, not newer or older. These versions are not widely packaged so a local install is necessary. (jaegertracing#45) - Requires nlohmann json 2.1.0 or newer, which is not widely packaged in Linux distros. Install a local copy. (jaegertracing#47) Signed-off-by: Craig Ringer <craig@2ndquadrant.com>

Improves jaegertracing#33 Documents jaegertracing#38, jaegertracing#45, jaegertracing#47, jaegertracing#51 Signed-off-by: Craig Ringer <craig@2ndquadrant.com>

…to local-dependencies-support

ringerc · 2018-02-13T06:51:20Z

@isaachier I merged the Travis files as requested

isaachier · 2018-02-13T22:41:20Z

I owe you a full review soon. Will get around to it ASAP.

ringerc · 2018-02-14T01:48:19Z

On 14 February 2018 at 06:37, isaachier ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In CMakeLists.txt <#46 (comment)> : > if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU" AND CMAKE_CXX_COMPILER_VERSION VERSION_LESS "4.9") set(boost_components regex) hunter_add_package(Boost COMPONENTS regex) - list(APPEND LIBS Boost::regex) So the reason for that is the header-only vs. library dependencies in Boost. Hunter needs to know which libraries it is going to build. I'd rather not build or use the regex library from Boost if standard C++11 regex is available.

Ah, I misunderstood it then. I read it as something like "on gcc < 4.9 we need to link to something to use Boost::regex; on later versions we can use it as a header-only library". Reading comprehension fail. Ah, hence regex_namespace in utils/Regex.h . I'll rewrite that a little if you're OK with it, so that CMake injects a macro into Constants.h to set JAEGERTRACING_USE_STD_REGEX rather than making the decision in utils/Regex.h. That way the intent should be clearer.

…

-- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services

isaachier · 2018-02-14T02:15:56Z

That makes sense, but the issue is that a team at Uber uses Bazel for their build, not CMake, and the fix is primarily for them. Not sure if they can use a CMake based macro. Then again, I think they hack a lot of it at the moment to get it working so not a huge deal.

ringerc · 2018-02-14T03:02:44Z

On 14 February 2018 at 10:15, isaachier ***@***.***> wrote: That makes sense, but the issue is that a team at Uber uses Bazel <https://bazel.build> for their build, not CMake, and the fix is primarily for them. Not sure if they can use a CMake based macro. Then again, I think they hack a lot of it at the moment to get it working so not a huge deal.

They wouldn't need to, it'd be injected into Config.h . Unless you mean that they build jaegertracing its self with Bazel, in which case they're already duplicating the whole build system. PostgreSQL uses autotools (sigh), so I'm not exactly keen to add hard CMake dependencies myself.

…

-- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services

isaachier · 2018-02-14T03:56:52Z

Right lol I get that. Ironically, Hunter supports autotools and maintains a PostgreSQL build, so you could probably leverage that somehow in a worst case scenario. Either way, my support for GCC 4.8 and below is declining. nlohmann specifically won't work on 4.8. There are some patches the team I work with used but I'd recommend not using them.

ringerc · 2018-02-14T05:36:21Z

On 14 February 2018 at 11:56, isaachier ***@***.***> wrote: Right lol I get that. Ironically, Hunter supports autotools and maintains a PostgreSQL build, so you could probably leverage that somehow in a worst case scenario. Either way, my support for GCC 4.8 and below is declining. nlohmann specifically won't work on 4.8. There are some patches the team I work with used but I'd recommend not using them.

Yeah. 4.8 isn't so old, though; notably RHEL 7 has 4.8 and RHEL6 has 4.4. That means I won't be able to deploy tracing on pretty much any customer system. I think I have to drop this. Between the Thrift issues, nlohmann version, etc, and the implied gcc dependency, I can't actually *use it* anywhere.

…

-- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services

isaachier · 2018-02-14T09:01:12Z

Ya I'm sorry again and not entirely surprised. This library was built for a very modern C++ application. That's why I've been working on a C version - - to reduce dependencies and increase portability.

ringerc · 2018-02-15T02:01:15Z

Thanks, and sorry I'm not actively contributing to the C one yet. I've burned all my time and then some trying to fix the C++ one up for my needs / for general portability. I can hack around with a specific Thrift version + installing a newer gcc or icc or clang, albeit painfully. Most likely I'd build packages and ship them with a new libstdc++. In the mean time it remains useful for development & debugging. I'll try to get work to dedicate some time/resources to the C backend but unsure how I'll go.

isaachier · 2018-02-15T02:07:29Z

@ringerc no worries. Eventually, I hope to remove much of the C++ internals from the C++ client, wrapping the C core instead. In that case, your code would work fine, even if not using C directly. When it gets to that level of maturity, I'll be sure to let you know.

ringerc · 2018-02-15T02:09:34Z

Good to know. I'll try to pick up the C client and do some testing in a bit. I'd rather adopt it than wrap the C client in a C++ API then wrap the C++ API in a C one for the app.

isaachier

@ringerc I think this change is a bit outdated and quite large. I am sorry I have not reviewed it adequately. If you do intend to change this, please update pull request and reopen it once you do. I suggest keeping the CMake build in one file if you don't mind. In my opinion, splitting the CMake file into different directories works best when the build is independent in each directory and the libraries require different options. Here, I don't see that being the case.

even after json package installed using distro package, cmake is not able to search for it's cmake config, adding a find_package method ensures garanteed check on finding dependency, which is either skipped or not discovered. Reference: CMake Error at CMakeLists.txt:80 (find_package): Could not find a package configuration file provided by "nlohmann_json" with any of the following names: nlohmann_jsonConfig.cmake nlohmann_json-config.cmake Add the installation prefix of "nlohmann_json" to CMAKE_PREFIX_PATH or set "nlohmann_json_DIR" to a directory containing one of the above files. If "nlohmann_json" provides a separate development package or SDK, be sure it has been installed. Adapted from: jaegertracing#46 Signed-off-by: Deepika Upadhyay <dupadhya@redhat.com>

ringerc force-pushed the local-dependencies-support branch from 524e9bb to 7f7f488 Compare January 28, 2018 12:52

ringerc force-pushed the local-dependencies-support branch from 7f7f488 to 28b16fd Compare January 28, 2018 13:11

isaachier reviewed Jan 28, 2018

View reviewed changes

ringerc force-pushed the local-dependencies-support branch from 28b16fd to 9c58195 Compare January 28, 2018 13:13

Isaac Hier added 2 commits January 28, 2018 08:25

Add GCC 7 and non-Hunter build to Travis

ddd6307

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

Fix matrix

08c9c65

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

ringerc force-pushed the local-dependencies-support branch 4 times, most recently from ea04e69 to 12990de Compare January 31, 2018 06:15

ringerc force-pushed the local-dependencies-support branch from 12990de to c5cf3d3 Compare February 1, 2018 04:05

ringerc force-pushed the local-dependencies-support branch from c5cf3d3 to e351ca3 Compare February 1, 2018 04:07

ringerc force-pushed the local-dependencies-support branch from c312dbc to 3a35900 Compare February 1, 2018 05:43

ringerc force-pushed the local-dependencies-support branch from 3a35900 to 379dc40 Compare February 1, 2018 06:10

ringerc force-pushed the local-dependencies-support branch 2 times, most recently from cf01f4c to 791c136 Compare February 1, 2018 07:50

isaachier reviewed Feb 1, 2018

View reviewed changes

ringerc force-pushed the local-dependencies-support branch from 791c136 to d7c93bf Compare February 1, 2018 15:19

ringerc force-pushed the local-dependencies-support branch 2 times, most recently from 95ba04e to 0d999a0 Compare February 2, 2018 04:17

ringerc added 3 commits February 13, 2018 14:49

Document library build/install and basic usage

2bb946b

Improves jaegertracing#33 Documents jaegertracing#38, jaegertracing#45, jaegertracing#47, jaegertracing#51 Signed-off-by: Craig Ringer <craig@2ndquadrant.com>

Merge remote-tracking branch 'remotes/isaachier/add-travis-builds' in…

2022642

…to local-dependencies-support

ringerc force-pushed the local-dependencies-support branch from 56e7996 to 2022642 Compare February 13, 2018 06:50

isaachier suggested changes Mar 26, 2018

View reviewed changes

isaachier closed this Mar 26, 2018

isaachier mentioned this pull request Mar 26, 2018

Add GCC 7 and non-Hunter build to Travis #48

Closed

isaachier mentioned this pull request Apr 5, 2018

Improve documentation #33

Closed

ideepika mentioned this pull request Mar 20, 2020

add Findnlohmann_json #219

Closed

Support building with local dependencies (no Hunter) #46

Support building with local dependencies (no Hunter) #46

Conversation

ringerc commented Jan 28, 2018

ringerc commented Jan 28, 2018

isaachier left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ringerc commented Jan 28, 2018 • edited Loading

ringerc commented Jan 29, 2018 via email

ringerc commented Jan 31, 2018

ringerc commented Feb 1, 2018

ringerc commented Feb 1, 2018 • edited Loading

ringerc commented Feb 1, 2018

ringerc commented Feb 1, 2018

codecov bot commented Feb 1, 2018 • edited Loading

Codecov Report

ringerc commented Feb 1, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

isaachier left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

isaachier Feb 13, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ringerc commented Feb 1, 2018 • edited Loading

ringerc commented Feb 7, 2018

isaachier commented Feb 12, 2018

ringerc commented Feb 13, 2018

isaachier commented Feb 13, 2018

ringerc commented Feb 14, 2018 via email

isaachier commented Feb 14, 2018

ringerc commented Feb 14, 2018 via email

isaachier commented Feb 14, 2018

ringerc commented Feb 14, 2018 via email

isaachier commented Feb 14, 2018

ringerc commented Feb 15, 2018 via email

isaachier commented Feb 15, 2018

ringerc commented Feb 15, 2018 via email

isaachier left a comment

Choose a reason for hiding this comment

ringerc commented Jan 28, 2018 •

edited

Loading

ringerc commented Feb 1, 2018 •

edited

Loading

codecov bot commented Feb 1, 2018 •

edited

Loading

ringerc commented Feb 1, 2018 •

edited

Loading

isaachier Feb 13, 2018 •

edited

Loading

ringerc commented Feb 1, 2018 •

edited

Loading