-
Notifications
You must be signed in to change notification settings - Fork 438
Correct crate versions and make CI semver test run on all crates (esp lightning-invoice)
#4375
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
Correct crate versions and make CI semver test run on all crates (esp lightning-invoice)
#4375
Conversation
|
I've assigned @wpaulino as a reviewer! |
48f1245 to
8691cba
Compare
The semver CI check is great but only checks the immediate crate in question. It doesn't catch that many of our crates depend on `lightning` and thus have actually broken semver as the types they use have changed to `lightning` 0.3. Here we hump the version of crates that have actually changed semver since 0.2. In addition to those that depend on `lightning`, `lightning-invoice`'s API has changed (but was not being checked by the semver CI task). Finally, `lightning-macros` was updated to 0.2.1, so the version is changed to 0.2.2.
21d443f to
19c29df
Compare
19c29df to
a123cfa
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4375 +/- ##
==========================================
- Coverage 86.01% 86.00% -0.02%
==========================================
Files 156 156
Lines 102781 102781
Branches 102781 102781
==========================================
- Hits 88409 88394 -15
- Misses 11864 11878 +14
- Partials 2508 2509 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
👋 The first review has been submitted! Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer. |
| with: | ||
| manifest-path: lightning-transaction-sync/Cargo.toml | ||
| feature-group: only-explicit-features | ||
| features: esplora-blocking |
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.
No, we did this intentionally for good reason, as esplora-blocking and esplora-async are mutually exclusive for example. If you enable both, you end up just checking the async variant. I think at the time there were more features like this, e.g., BP's futures.
| manifest-path: lightning-transaction-sync/Cargo.toml | ||
| feature-group: only-explicit-features | ||
| features: esplora-async | ||
| - name: Install Rust stable toolchain |
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.
Why not just stick with the appropriate Github action rather than doing something custom?
|
Now opened #4378 to revert the last commit, let's discuss there what parts of the changes still make sense. |
No description provided.