-
Notifications
You must be signed in to change notification settings - Fork 8
Bump MSRV and reqwest dependency
#38
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
|
👋 Thanks for assigning @tankyleo as a reviewer! |
d886342 to
455e666
Compare
455e666 to
c5599bc
Compare
reqwest, enable client-side timeoutsreqwest dependency
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.
Thank you ! Do we want RUSTFLAGS=--cfg=genproto cargo +1.75.0 build to work ? Otherwise LGTM
Hmm, not sure tbh? Do you think we should? Also, should we add a CI that runs |
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.
Hmm, not sure tbh? Do you think we should?
Upon further thought, I don't think it's necessary since this is a developer command; first update API definition, then update src/types.rs.
Also, should we add a CI that runs genproto and checks the output is up-to-date, i.e., requiring any PRs going forward to update the types.rs as we go, similar to running rustfmt?
Certainly yes - I'll take care of this in a follow-up thank you.
We bump the MSRV to 1.75.0 as most of our HTTP dependencies already require this, and we already require it for
lightning-transaction-sync/ LDK Node.As we can do this then, we bump the
reqwestdependency to 0.12 (which is nice as most other projects have by now, allowing us to not keep multiple versions in the tree).