Skip to content

Housekeeping: fix some warning and docs #2568

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 5 commits into from
Sep 14, 2023

Conversation

tnull
Copy link
Contributor

@tnull tnull commented Sep 12, 2023

We fix how we expose ChannelId as it looked weird in docs and was reachable via multiple ways.

Moreover, we fix a bunch of compiler warnings.

@tnull tnull force-pushed the 2023-09-housekeeping branch from 9e72be3 to 8f403c0 Compare September 12, 2023 11:58
@TheBlueMatt
Copy link
Collaborator

Oops, I had a few of these queued up in the first commit on #2169. Happy to rebase on here or whatever, though...I really need to stop slipping cleanups in PRs I think are gonna be merged soon (tm) :/

wpaulino
wpaulino previously approved these changes Sep 12, 2023
@valentinewallace
Copy link
Contributor

I'm still getting 2 warnings on this branch, are those removable at this time?

`ChannelId` was weirdly listed in the re-export section of the docs and
reachable via multiple paths. Here we opt to make the `channel_id`
module private and leave only the `ChannelId` struct itself exposed.
@tnull tnull force-pushed the 2023-09-housekeeping branch from 58554ce to 2ece4dd Compare September 13, 2023 07:50
@tnull
Copy link
Contributor Author

tnull commented Sep 13, 2023

Oops, I had a few of these queued up in the first commit on #2169. Happy to rebase on here or whatever, though...I really need to stop slipping cleanups in PRs I think are gonna be merged soon (tm) :/

Ah, did see that some of them were fixed there already. Now rebased after it was merged.

I'm still getting 2 warnings on this branch, are those removable at this time?

I addressed most of them now, only hesitant to touch the unused fn as_mut_ecdsa(&mut self) in type_resolver.rs as I'm not sure if it's meant to be used in a follow-up somewhere.

@TheBlueMatt
Copy link
Collaborator

I addressed most of them now, only hesitant to touch the unused fn as_mut_ecdsa(&mut self) in type_resolver.rs as I'm not sure if it's meant to be used in a follow-up somewhere.

It is, yes.

TheBlueMatt
TheBlueMatt previously approved these changes Sep 13, 2023
@tnull
Copy link
Contributor Author

tnull commented Sep 14, 2023

Force-pushed once more including the following fixups:

> git diff-tree -U2 2ece4dd0 411a3f7d
diff --git a/lightning/src/ln/monitor_tests.rs b/lightning/src/ln/monitor_tests.rs
index 95229348..ece24794 100644
--- a/lightning/src/ln/monitor_tests.rs
+++ b/lightning/src/ln/monitor_tests.rs
@@ -2232,5 +2232,5 @@ fn test_anchors_aggregated_revoked_htlc_tx() {
        let spendable_output_events = nodes[0].chain_monitor.chain_monitor.get_and_clear_pending_events();
        assert_eq!(spendable_output_events.len(), 2);
-       for (_idx, event) in spendable_output_events.iter().enumerate() {
+       for event in spendable_output_events.iter() {
                if let Event::SpendableOutputs { outputs, channel_id } = event {
                        assert_eq!(outputs.len(), 1);
diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs
index 63b33a67..785b40be 100644
--- a/lightning/src/util/test_utils.rs
+++ b/lightning/src/util/test_utils.rs
@@ -283,5 +283,5 @@ struct JusticeTxData {
 }

-pub struct WatchtowerPersister {
+pub(crate) struct WatchtowerPersister {
        persister: TestPersister,
        /// Upon a new commitment_signed, we'll get a

@TheBlueMatt TheBlueMatt merged commit 24db35e into lightningdevkit:main Sep 14, 2023
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.

5 participants