Use cpp17 and no need for Boost if you disable examples. #284
Conversation
…se it was useless actually.
TheSharpOwl
changed the title
Edit: if you don't want to break compatibility, I am planning to add which C++ to use for example so that it would be available in old way or new one ? |
|
Given this is quite the breaking change, it should be an optional feature that we can eventually enable by default in say version 1.0. Using Could you turn this into a configurable option? e.g. |
|
Sure then I will do it and let you know)) Thanks. |
|
I did it and if someone wants to use only C++17 without any boost, they should do this: |
| endif() | ||
| endif() | ||
|
|
||
| if(USE_CPP17) | ||
| set(CMAKE_CXX_STANDARD 17) | ||
| add_definitions("-D_USE_CPP17") |
There was a problem hiding this comment.
I don't think this is enough. There needs to be a record somewhere (e.g. in a generated header file) that stores this and code should include that file to figure out this flag.
If you don't do that, you can have the library being compiled with this flag on but an application that uses the library not knowing (for obvious reasons) that they need to set this macro manually, which will break badly because from one's perspective you're using std types and from the other's boost ones.
| @@ -27,6 +27,10 @@ | |||
| * | |||
| */ | |||
|
|
|||
| #ifdef _WIN32 | |||
| #define NOMINMAX | |||
There was a problem hiding this comment.
This should probably be set as a definition at the cmake level if it's needed, rather than here in this particular file
| @@ -143,7 +143,11 @@ void Producer::do_produce(const Message& message, | |||
| const Buffer& payload = message.get_payload(); | |||
| const Buffer& key = message.get_key(); | |||
| const int policy = static_cast<int>(message_payload_policy_); | |||
| #ifdef _USE_CPP17 | |||
| int64_t duration = message.get_timestamp() ? message.get_timestamp().value().get_timestamp().count() : 0; | |||
There was a problem hiding this comment.
I presume both optionals have operator* so rather than doing this you can maybe use that which should make the same expression work for both types?
I replaced the boost::optional and boost::any with C++17 ones (project is set to use C++17). However, examples still use boost and I have no idea yet how to refactor that. Why I did it ? To save space if someone just wants to install the library.