Skip to content

Add is_running function to Node - #169

Merged
tnull merged 1 commit into
lightningdevkit:mainfrom
jbesraa:feat/is-node-running
Oct 5, 2023
Merged

Add is_running function to Node#169
tnull merged 1 commit into
lightningdevkit:mainfrom
jbesraa:feat/is-node-running

Conversation

@jbesraa

Copy link
Copy Markdown
Contributor

resolves#164

Comment threadsrc/lib.rs Outdated
Ok(())
}

/// Checks if [`Node`] runtime is available.

@jbesraajbesraaSep 27, 2023

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

indentation is a bit off, will fix it.

anything else I should mention in the comment?
because this check is more "codey" check and not really has anything to do with the network, should we mention this?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks! Seems fine, although we could maybe just make it "Returns whether the [Node] is running." (as mentioning the runtime feels a bit like we're leaking internals that the user doesn't need to care about).

@tnulltnull left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM, can we also expose that in bindings, i.e., add it to ldk_node.udl?

 - Expose in bindings
@jbesraa
jbesraaforce-pushed the feat/is-node-running branch from 0212d21 to 0a95d72CompareOctober 2, 2023 11:19
@jbesraa

Copy link
Copy Markdown
ContributorAuthor

looks like we need to pin some crate(s) here?

@tnull

tnull commented Oct 5, 2023

Copy link
Copy Markdown
Collaborator

looks like we need to pin some crate(s) here?

Yeah, fixed that in #151.

@tnull
tnull merged commit 0c13726 into lightningdevkit:mainOct 5, 2023
@jbesraa
jbesraa deleted the feat/is-node-running branch October 24, 2023 06:17
Sign up for freeto 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.

Check if node is running

2 participants

@jbesraa@tnull