Skip to content

Fix: Move platform-agnostic files outside of LIBCORO_FEATURE_THREADING#175

Merged
jbaldwin merged 7 commits intojbaldwin:mainfrom
bjones1:threading-refactor
Oct 12, 2023
Merged

Fix: Move platform-agnostic files outside of LIBCORO_FEATURE_THREADING#175
jbaldwin merged 7 commits intojbaldwin:mainfrom
bjones1:threading-refactor

Conversation

@bjones1
Copy link
Contributor

@bjones1 bjones1 commented Sep 30, 2023

This implements the suggestion in #170.

@bjones1 bjones1 marked this pull request as ready for review September 30, 2023 01:42
@bjones1
Copy link
Contributor Author

bjones1 commented Oct 7, 2023

Do you want me to merge in the main branch, or rebase this branch to main? I'm happy to, just let me know your workflow.

Is there anything else to do on this PR?

@jbaldwin
Copy link
Owner

jbaldwin commented Oct 9, 2023

Generally whatever works best, I usually only squash and rebase into main so it's a singular commit in the final git log.

@bjones1
Copy link
Contributor Author

bjones1 commented Oct 10, 2023

Looks like you merged main in successfully -- thanks! Do you need anything else from me on this PR?

I'm excited to get this merged; I think this will get us to successful builds on XCode and MSVC. I'll do some more testing/cleanup after the merge.

@jbaldwin
Copy link
Owner

I was planning on merging this tonight, but I did take one last look at it and I do think some of the test files can probably be moved outside of the feature flag pretty easily by switching the tests from coro::io_scheduler to coro::thread_pool. Unless if there is another reason you included them inside the feature flag? Would you mind taking a look and letting me know what you think.

…depend on LIBCORO_FEATURE_THREADING.

However, this makes the test run a LOT slower.
@jbaldwin jbaldwin merged commit cef2ff0 into jbaldwin:main Oct 12, 2023
@bjones1 bjones1 deleted the threading-refactor branch October 12, 2023 12:29
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.

2 participants