-
Notifications
You must be signed in to change notification settings - Fork 119
Add commented-out patch section for LDK Git dependencies
#764
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
Add commented-out patch section for LDK Git dependencies
#764
Conversation
For convenience we added ready-to-go `patch` setions to `Cargo.toml`, which however only allowed us to quickly override official LDK releases. Here we also add the corrsponding counterparts for LDK `git` dependencies.
|
👋 Thanks for assigning @joostjager as a reviewer! |
| #vss-client-ng = { path = "../vss-client" } | ||
| #vss-client-ng = { git = "https://github.com/lightningdevkit/vss-client", branch = "main" } | ||
| # | ||
| #[patch."https://github.com/lightningdevkit/rust-lightning"] |
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.
With multiple patch sections, idk if it is still better than just having a commented-out section in the regular location (no patch)?
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.
It is. Note that patch is fundamentally different in that it will transitively patch the dependency for all sub-dependencies. So if you patch in LDK Node, you won't need to have it point to a changed version of bitcoin-payments-instruction also pointing to the same LDK dependency, etc.
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.
Ah, good to know. I guess it's arguable how useful the crates-io patch still is. Because very soon after an LDK release, we'll probably bump to an ldk latest commit again.
In the last few days there was incompatibility of `cargo-semver-checks` with the new stable Rust 1.93.0. While this should fixed by today's release of `cargo-semver-checks`, we take the opportunity to drop an unnecessary install step from the CI workflow, as the action will bring their own Rust version if not configured otherwise.
|
Now also included a commit that drops an unnecessary step from the SemVer CI workflow. |
|
🔔 1st Reminder Hey @joostjager! This PR has been waiting for your review. |
| #vss-client-ng = { path = "../vss-client" } | ||
| #vss-client-ng = { git = "https://github.com/lightningdevkit/vss-client", branch = "main" } | ||
| # | ||
| #[patch."https://github.com/lightningdevkit/rust-lightning"] |
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.
Ah, good to know. I guess it's arguable how useful the crates-io patch still is. Because very soon after an LDK release, we'll probably bump to an ldk latest commit again.
Fixes #758.
For convenience we added ready-to-go
patchsetions toCargo.toml, which however only allowed us to quickly override official LDK releases.Here we also add the corrsponding counterparts for LDK
gitdependencies.