-
Notifications
You must be signed in to change notification settings - Fork 62
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
fix: error event propagation #233
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,8 @@ yarn.lock | |
**/*.log | ||
test/repo-tests* | ||
|
||
.vscode | ||
.eslintrc | ||
# Logs | ||
logs | ||
*.log | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -130,6 +130,7 @@ class FactoryInProc { | |
} | ||
|
||
const node = new Node(options) | ||
node.once('error', err => callback(err, node)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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), | ||
|
There was a problem hiding this comment.
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.There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.There was a problem hiding this comment.
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 😄