Skip to content
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

Jump to bottom

fix: error event propagation #233

Merged
merged 2 commits into from
May 4, 2018
Merged

fix: error event propagation #233

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7b412ac
fix: error event propagation
hugomrdias Apr 27, 2018
8c2ff7d
fix: register error event first
hugomrdias May 4, 2018
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 .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ yarn.lock
**/*.log
test/repo-tests*

.vscode
.eslintrc
Copy link
Member

Choose a reason for hiding this comment

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

Is there any reason we need to ignore the . eslintrc file if its not being used? Usually aegir takes care of all our configs, linting included.

Copy link
Member Author

Choose a reason for hiding this comment

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

editors need to find a eslint config to provide linter messages and fix problems, because the config is hidden inside the aegir > eslint-config-aegir > .eslintrc the editor cant find it.
because of this i need to manually add this file in every repo to have a normal ctrl-s workflow.

Copy link
Member

Choose a reason for hiding this comment

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

Editors will usually allow pointing their linting tools to custom locations, and with many contributors/editors being used I don't think it would be practical to add a custom entry for each to the .gitignore file.

I don't know what our current guidelines in regards to .gitignore are, maybe @diasdavid or @victorbjelkholm can provide more context.

Copy link
Member Author

Choose a reason for hiding this comment

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

This works for all editors because this is all based on how eslint works, nothing fancy here 😄

# Logs
logs
*.log
Expand Down
1 change: 1 addition & 0 deletions src/factory-in-proc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ class FactoryInProc {
}

const node = new Node(options)
node.once('error', err => callback(err, node))
Copy link

@Mr0grog Mr0grog May 2, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

If a node emits an error after starting, won’t this result in spawn’s callback getting called more than once? (First with a success, then, later, with an error.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Shouldn't happen but ur right error should come first. Good catch, thank you


series([
(cb) => node.once('ready', cb),
Expand Down
1 change: 1 addition & 0 deletions src/ipfsd-in-proc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ class Node extends EventEmitter {
libp2p: this.opts.libp2p
})

this.exec.once('error', err => this.emit('error', err))
this.exec.once('ready', () => this.emit('ready'))
}

Expand Down