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

Made CMakeLists.txt CPM-compatible #254

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

Conversation

sdeleon28
Copy link

Implemented the fix detailed in #253

In summary this PR:

  • Makes it possible to include react-juce with CPM
  • Makes it possible to modify REACTJUCE_JS_LIBRARY from client code
  • Makes building JUCE optional
  • Makes building the example optional
  • Documents all of these changes

@sdeleon28
Copy link
Author

I'll fix that linting error in a sec

@JoshMarler
Copy link
Owner

@sdeleon28 , apologies for the delay on this one. In crunch time at current shop. I'll do my utmost to take a first pass over this tonight!

@mjmaurer
Copy link
Contributor

Any updates here? Would love to have this merged :)

@nick-thompson
Copy link
Collaborator

I'm for it, but we had originally held off on merging this because we had come up with a different way that we wanted to structure our CMake here. In particular, I remember @JoshMarler suggesting that the root level project shouldn't add juce, that we can delegate that to whomever calls add_subdirectory(react_juce), and therefore we can move the juce inclusion here into the example project. I think that structure would clean things up a bit still, so I think I'd prefer that. Am I missing anything there @JoshMarler? Was there more to it?

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.

4 participants