Skip to content

Fix several compile warnings added in some of my recent commits #1710

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

Merged
merged 3 commits into from
Sep 11, 2022

Conversation

TheBlueMatt
Copy link
Collaborator

No description provided.

@valentinewallace
Copy link
Contributor

I see another warning in beta if we wanna fix that here

@TheBlueMatt
Copy link
Collaborator Author

warning: unused return value of std::boxed::Box::<T>::from_raw that must be used

Lol, what a dumb warning, from_raw as a way to drop a pointer is a common pattern, why would you warn on a common pattern!

@codecov-commenter
Copy link

codecov-commenter commented Sep 9, 2022

Codecov Report

Merging #1710 (042b62b) into main (3fb3218) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 042b62b differs from pull request most recent head 93291f9. Consider uploading reports for the commit 93291f9 to get more accurate results

@@            Coverage Diff             @@
##             main    #1710      +/-   ##
==========================================
- Coverage   90.84%   90.83%   -0.01%     
==========================================
  Files          86       86              
  Lines       46399    46398       -1     
  Branches    46399    46398       -1     
==========================================
- Hits        42150    42145       -5     
- Misses       4249     4253       +4     
Impacted Files Coverage Δ
lightning/src/ln/channelmanager.rs 85.00% <ø> (ø)
lightning/src/routing/gossip.rs 91.42% <100.00%> (-0.01%) ⬇️
lightning/src/util/test_utils.rs 77.55% <100.00%> (ø)
lightning/src/util/wakers.rs 86.70% <100.00%> (ø)
lightning-invoice/src/utils.rs 96.63% <0.00%> (-0.15%) ⬇️
lightning/src/ln/monitor_tests.rs 99.44% <0.00%> (-0.12%) ⬇️
lightning/src/ln/functional_tests.rs 96.81% <0.00%> (-0.08%) ⬇️
lightning/src/chain/onchaintx.rs 94.71% <0.00%> (+0.68%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

wpaulino
wpaulino previously approved these changes Sep 9, 2022
Copy link
Contributor

@wpaulino wpaulino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's wait to merge this until we have all 0.0.111 items completed so we can rebase and make sure no other warnings slip in?

@TheBlueMatt
Copy link
Collaborator Author

Rebased on all the 111 changes. Should be good to go now.

@TheBlueMatt TheBlueMatt added this to the 0.0.111 milestone Sep 9, 2022
@TheBlueMatt TheBlueMatt mentioned this pull request Sep 9, 2022
jkczyz
jkczyz previously approved these changes Sep 9, 2022
@wpaulino
Copy link
Contributor

wpaulino commented Sep 9, 2022

There are a few more on the stable & beta no-std builds:

warning: unused import: `core::time::Duration`
  --> lightning/src/util/wakers.rs:18:5
   |
18 | use core::time::Duration;
   |     ^^^^^^^^^^^^^^^^^^^^

warning: field `full_syncs_requested` is never read
   --> lightning/src/routing/gossip.rs:200:2
    |
195 | pub struct P2PGossipSync<G: Deref<Target=NetworkGraph<L>>, C: Deref, L: Deref>
    |            ------------- field in this struct
...
200 |     full_syncs_requested: AtomicUsize,
    |     ^^^^^^^^^^^^^^^^^^^^

warning: associated function `should_request_full_sync` is never used
   --> lightning/src/routing/gossip.rs:239:5
    |
239 |     fn should_request_full_sync(&self, _node_id: &PublicKey) -> bool {

warning: constant `FULL_SYNCS_TO_REQUEST` is never used
   --> lightning/src/routing/gossip.rs:241:3
    |
241 |         const FULL_SYNCS_TO_REQUEST: usize = 5;

warning: unused import: `InitFeatures`
    --> lightning/src/routing/gossip.rs:1868:38
     |
1868 |     use ln::features::{ChannelFeatures, InitFeatures, NodeFeatures};
     |                                         ^^^^^^^^^^^^

@TheBlueMatt
Copy link
Collaborator Author

Oops, looks like half of those were introduced when I fixed other issues. For some reason the no-std warnings aren't reproducing locally so i was relying on CI.

@TheBlueMatt
Copy link
Collaborator Author

I'm just gonna leave the InitFeatures one for #1707, its test-only anyway.

@TheBlueMatt TheBlueMatt dismissed stale reviews from valentinewallace and jkczyz via 042b62b September 9, 2022 23:30
@TheBlueMatt TheBlueMatt force-pushed the 2022-09-compile-warn branch 2 times, most recently from 042b62b to 93291f9 Compare September 9, 2022 23:52
@TheBlueMatt TheBlueMatt merged commit 15a5966 into lightningdevkit:main Sep 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants