grantila / q

A platform-independent promise library for C++, implementing asynchronous continuations.
http://libq.io
Apache License 2.0
193 stars 24 forks source link

start with some cmake refactoring, early preview #2

Closed a4z closed 7 years ago

a4z commented 7 years ago

this is an early preview with the first step, maybe not to merge now the windows build should be unchanged, i tested gcc and clang on linux, no apple availalbe

please review, look at the comments, test if possible, and provide feedback, thanks! based on that I con continue with additional tasks, maybe togeteer with using the issue tracker to discuss actions upfront.

grantila commented 7 years ago

Thanks for the work! This looks generally good, I moved it to another branch cmake-refactoring so that it can be worked on out-of-master.

The Trompeloeil suggestion isn't bad, header-only has its benefit, however, this is used for q-test which is a promisified version of gtest (without the mocks i.e). So, this is more what I use as a base testing tool underneath q-test to drive the unit tests, provide nice command line arguments and output, and also to allow people to continue to use gtest if they wish (and most people seem to use gtest)... It could make sense to make two versions of q-test. One depending on gtest, one on trompeloeil, but not right now ;)

However, I think gtest/gmock could be depended on like any other library instead of building it inline with the rest of the code. This provides some of the background about why it was inlined: http://stackoverflow.com/questions/24917260/how-to-build-gtest-within-project-and-use-find-packagegtest-and-gtest-add-test/24922055#24922055 Also, on Mac using brew install gtest you get this lovely response: Installing gtest system-wide is not recommended; it should be vendored in your projects that use it. And Google's argument is which explains the reasoning is this: https://github.com/google/googletest/blob/master/googletest/docs/FAQ.md#why-is-it-not-recommended-to-install-a-pre-compiled-copy-of-google-test-for-example-into-usrlocal So it looks like an in-repo build after all..

I'll go ahead and work a bit in this too, and see how well it works on the other platforms