Skip to content
This repository was archived by the owner on Jul 10, 2025. It is now read-only.

Conversation

DieMyst
Copy link
Contributor

@DieMyst DieMyst commented Feb 18, 2021

No description provided.

@DieMyst DieMyst requested a review from coder11 February 18, 2021 11:17
Copy link
Contributor

@coder11 coder11 left a comment

Choose a reason for hiding this comment

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

Please take a look on the comments

src/api.ts Outdated
}
return true;
} catch (e) {
console.error("Error on establishing connection: ", e)
Copy link
Contributor

Choose a reason for hiding this comment

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

log instead of console

src/api.ts Outdated
3000
);

if (client.isConnected) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefer inverting condition to reduce nesting

src/api.ts Outdated
connectTo?: string | Multiaddr | Node,
peerIdOrSeed?: PeerId | string,
): Promise<FluenceClient> => {
): Promise<FluenceClientImpl> => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Exposing FluenceClient is intentional. We probably don't want to expose internals to public api

@coder11 coder11 merged commit c10cd19 into master Feb 19, 2021
@coder11 coder11 deleted the check-connection branch March 16, 2021 08:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants