Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/add cmake support #35

Open
wants to merge 25 commits into
base: master
Choose a base branch
from

Conversation

ClausKlein
Copy link

To be portable, I had to use catch2 instead of CppUnitTestFramework

captaincrutches and others added 12 commits November 19, 2021 14:11
This prevents a hang and/or segfault when trying to lock a moved-from observer.

When moving an observer, we now do the equivalent of a disconnect_all on the source object, and also re-wire those connections onto the target object:
- Insert all connections previously on the source object into the target
- On all connected observers, disconnect them from the source object and re-connect to the target
…, disconnect_all is a noop and we don't need movedFrom
…onstructors

Explicitly default/delete copy and move constructors for signal and observer types
Enhanced the move tests and resolved uncovered issues in move_connections_from.
Normalized method names, default statement locations, comments.
use CPM for booststap
use catch2 for unit tests, not yet finisched!
@ClausKlein ClausKlein marked this pull request as draft March 3, 2022 12:16
@ClausKlein ClausKlein marked this pull request as ready for review March 3, 2022 22:41
@NoAvailableAlias
Copy link
Owner

Thank you for this PR as well as that opened issue (that is still to be reviewed...).
I'm going to checkout ClausKlein/nano-signal-slot to see what I must do to my own workflow.

The only thing I can say is to possibly add some text in tests/readme concerning the need for catch2 and a link to it.
This won't block the PR though, I'll add it post merge.
Sorry for being extremely tardy.

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.

3 participants