search

karastojko / mailio

mailio is a cross platform C++ library for MIME format and SMTP, POP3, IMAP protocols. It is based on the standard C++ 17 and Boost library.
Other
380 stars 101 forks source link

CMake Setup Suggestions #11

Closed DeveloperPaul123 closed 5 years ago

DeveloperPaul123 commented 6 years ago

First just want to say this is a great library, but I have some questions/suggestions regarding the project setup.

  1. Is there any particular reason why you have a static and dynamic build targets for the same library? Typically, you would allow the user/consumer of your library to decide if they want a shared library or a static lib. They could do this by setting the built in CMake variable BUILD_SHARED_LIBS.
  2. I had to move the cmake_policy call to the top level cmake lists to avoid the known bug with cmake 3.12 regard FindDoxygen
  3. Any reason why the examples you have in the repo can't be part of the project? They would be trivial to add via cmake.
  4. I would also suggest adding options to give the user explicit control over building test, examples and documentation. So adding options like MAILIO_BUILD_TESTS, MAILIO_BUILD_DOCUMENTATION, MAILIO_BUILD_EXAMPLES.
  5. The check for Boost_FOUND is probably not needed since it's required via CMake.
  6. I would suggest exporting your library so that it's easy to use in other CMake based projects.

Let me know what you think about the above items. If you're open to it I can file a PR to address all of these items.

karastojko commented 6 years ago

Hi Paul, Thank you for the feedback. I agree with you. One of the contributors was so kind to create the initial version of the CMake script. I have planned to add building examples, and definitely there are more space for improvements. If you don't mind, make a PR and then I will test it and merge it.

DeveloperPaul123 commented 6 years ago

Sounds good!

DeveloperPaul123 commented 5 years ago

@karastojko How do you feel about upgrading the minimum required version of CMake needed? I can really clean up and improve the setup greatly, but it will require some of the newer features available in the latest versions of CMake (probably somewhere between 3.8 and 3.10).

audaki commented 5 years ago

@DeveloperPaul123 @karastojko

I'm personally a "bleeding-edge" kind of dev, and since I'm one of the people who are using this library in a commercial production setting (shameless self-plug: https://firstbill.de , a German invoicing web-app in C++) I would like to give my opinion: Our production servers are already running cmake 3.10.2 (openSUSE Leap 15), so this would be okay for us.

But it might be a problem for others, see:

https://gitlab.kitware.com/cmake/community/wikis/doc/cmake/Life-Cycle-Considerations

karastojko commented 5 years ago

@DeveloperPaul123 @EarthlingKira I am fine with CMake 3.10, I've been using versions 3.10, 3.11 i 3.12 for a while. From the moment when Boost 1.66 became the minimal required version (starting at July), I have also upgraded CMake requirements. Anyway, the version 2.8 in CMakeLists.txt is put by the original contributor @TrevorMellon , so perhaps he could advice on this topic too.

DeveloperPaul123 commented 5 years ago

@karastojko I see that you have made some changes to the cmake setup. Do you prefer to keep everything in a single file?

karastojko commented 5 years ago

One file for the library and one for the examples is just fine. I am merging the PR to master.

DeveloperPaul123 commented 5 years ago

Awesome thanks!