Conversation
89740ac to
1b833be
Compare
dcbe3cf to
db9523e
Compare
|
Am I supposed to be reviewing or merging this? You might notice I switched to CPM for a few other dependancies on develop. Can this branch be pulled in line with that? |
|
@drowaudio I've been using this branch for a year as a dependency of my synth but never got the example plugins running in CI. I'll need to get that going and resolve recent conflicts before it's mergable. |
|
Ah ok. Poke me if/when you get to that. I might switch to juce via CPM at some point soon anyway... |
dc9e598 to
b596c78
Compare
| with: | ||
| path: ${{ env.PLUGIN_CACHE_PATH }} | ||
| # Increment the version in the key below to manually break plugin cache | ||
| key: v7-${{ runner.os }}-${{ env.JUCE_SHA1 }} |
There was a problem hiding this comment.
It's harder to grab this SHA1 when JUCE is coming from CPM. I looked at the history and decided this feature is unnecessary:
- In most cases it's fine to bump JUCE without clearing cache
- When bumping JUCE, the cache version can be manually bumped if desired
|
@drowaudio I merged in develop, aligned CPM, fixed the plugin tests and added tests specifically for this new type of usage (as a dependency of a JUCE project). I have been using pluginval this way, but there is one drawback for you/us as maintainers, which is noted in the description. I won't be offended if you don't want to take that additional burden on. |
|
I think if you're using it this way you have two options:
It's basically impossible to do anything else for libraries that don't maintain a stable API and ABI (which is very hard to do). |
|
I've been maintaining JUCE 7/8 compatibility with some other open source projects and I would classify it as "mildly annoying" (for example the JUCE 8 font changes). But yeah, I don't think pluginval itself needs to make any big promises. I haven't tested JUCE 7 and I noted in the README changes that it would be JUCE 8 only. |
Closes #137
This improves support for pluginval as a CMake sub-project in a JUCE project.
After this PR, all one needs to do is
add_subdirectory ("modules/pluginval")to a plugin project's CMakeLists.txt.PLUGINVAL_FETCH_JUCEoption (no longer needed, superseded by not being top level)To review, the benefits are:
Note:
Technically, this feature adds an additional maintenance burden to pluginval — before this change, pluginval only had to care about working for the single JUCE version it came bundled with. Now, it would need some preprocessor
#ifstatements to maintain some backwards compatibility with older JUCE versions (like I did in this PR for JUCE < 8.0.11)