Skip to content

node emits "online" event once online #93

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.

Sign up for GitHub

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

Closed
wants to merge 10 commits into from
Closed

node emits "online" event once online #93

Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
7b9bf40
emit online event once online
pgte Jun 30, 2017
606fa73
WIP - feat: convergence (#94)
daviddias Jul 4, 2017
547c4db
docs: refactor examples (#96)
daviddias Jul 5, 2017
13452c9
docs: rm bundles from table
daviddias Jul 5, 2017
2d40f3a
docs: update header of the README (#97)
daviddias Jul 5, 2017
2960861
isOn -> isStarted + start and stop events
pgte Jul 6, 2017
8c9973b
emit online event once online
pgte Jun 30, 2017
6d3c95b
isOn -> isStarted + start and stop events
pgte Jul 6, 2017
422513a
Merge branch 'feat/emit-online' of github.com:libp2p/js-libp2p into f…
pgte Jul 6, 2017
da9bff4
corrected isStarted
pgte Jul 6, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,8 @@ class Node extends EventEmitter {
return each(this.modules.discovery, (d, cb) => d.start(cb), cb)
}
cb()

this.emit('online')
Copy link
Member

Choose a reason for hiding this comment

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

I'm not convinced that online/offline is the right wording here. In js-ipfs we have:

image

start/stop sounds better because online has a lot of assumptions today. To me, start/stop say "hey, the thing you set up, is running".

Thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree, "online" is somewhat ambiguous, and not the goal. I'd say started and stopped are what are the really important events for the user (at the libp2p and ifs levels).

Following that trail, I think this leads to replacing .isOnline() with .isStarted(), no?

Copy link
Member

Choose a reason for hiding this comment

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

@pgte I like that :) (maybe isRunning?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps we should use the same verbs, otherwise it may get confusing to users, no?
The other alternative is to be explicit (in the docs) about the state machine (states) and the transitions (events). Example:

starting event -> starting state -> started event -> running state -> stopping event -> stopped state

Makes sense?

Copy link
Member

Choose a reason for hiding this comment

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

👍🏽

},
(cb) => {
if (this._dht) {
Expand Down