-
Notifications
You must be signed in to change notification settings - Fork 9
Parameterize dependencies to allow for easier testing against PRs, branches. #124
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
Conversation
mattldawson
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to me!
Should we add some developer instructions for how to point to a specific repo/commit, maybe in the README or documentation? Or we can create a separate issue for this.
I'm happy to write up a how-to/syntax overview, if you point me to where you'd like it. If you confirm the |
I think the README should be fine for now, since the codebase is still in early development. David set up some documentation that we could eventually move some of the README info to, but I think it will only be built with our first release. |
|
Sounds good, change incoming momentarily and then swapping context to other writing!! :) |
|
I put something in the |
Looks great! Thanks! |
|
Thanks! I'll merge, since both reviewers have approved it! |
I've parameterized the stanzas in
dependencies.cmake, to allow for easier testing against a branch/fork/PR without changing local files. This will reduce the overhead of keeping track of local changes if testing against a change in a dependency, and will reduce the chances of incorrect dependency versions leaking into the development branch.I've added a set of new environmental variables,
[DEPENDENCY]_GIT_REPOSITORYand[DEPENDENCY_GIT_TAG]. These are currently set to the default values inmain. They can be overridden when need be. For example:Instead of making these variables
options, and muddying the water, I've left them as environmental variables. I've cut down on a lot of redundant code by adding a function at the top ofdependencies.cmake,set_git_default().Feedback welcome! Hopefully this will prove useful, and it will save time on my end by letting me (and others) more easily test
musicaagainst changes in dependencies.