Skip to content

Conversation

@cdalke-havoc
Copy link

@cdalke-havoc cdalke-havoc commented Nov 8, 2024

Overview

This PR adds a "modern" CMake build setup similar to some of your other projects (Seqlock, MPMCQueue) which makes it easier to consume this in downstream projects using Cmake.

Some other changes:

  • Put TokenBucket under a rigtorp namespace.
  • Move headers to include and test source to src
  • Add the CI tests from MPMCQueue that test all platforms
  • Make tests compatible with windows

The CMakeFile is copied from MPMCQueue and has test + install targets.

Test target runs the test program:

> make test
Running tests...
Test project /home/cdalke/TokenBucket/build
    Start 1: TokenBucketTest
1/1 Test #1: TokenBucketTest ..................   Passed    1.00 sec

100% tests passed, 0 tests failed out of 1

Install target copies the headers:

> make install
Consolidate compiler generated dependencies of target TokenBucketTest
[100%] Built target TokenBucketTest
Install the project...
-- Install configuration: ""
-- Installing: /home/cdalke/TokenBucket/./install/include
-- Installing: /home/cdalke/TokenBucket/./install/include/rigtorp
-- Installing: /home/cdalke/TokenBucket/./install/include/rigtorp/TokenBucket.h
-- Installing: /home/cdalke/TokenBucket/./install/lib/cmake/TokenBucket/TokenBucketConfig.cmake
-- Installing: /home/cdalke/TokenBucket/./install/lib/cmake/TokenBucket/TokenBucketConfigVersion.cmake

The tests in Debug mode for Windows and Mac seem to hang which I suspect is a tooling problem, but I just turned those off.

@cdalke-havoc cdalke-havoc marked this pull request as ready for review November 8, 2024 18:38
@cdalke-havoc
Copy link
Author

Hi @rigtorp, thanks for making this library. This PR will make it a little easier to consume downstream via CMake's FetchContent on a few projects I'm involved with that consume this. Would appreciate your consideration!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant