-
Notifications
You must be signed in to change notification settings - Fork 30.5k
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
test: adjust indentation for stricter linting #14431
Changes from all commits
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 |
---|---|---|
|
@@ -212,13 +212,14 @@ class ActivityCollector { | |
} | ||
|
||
exports = module.exports = function initHooks({ | ||
oninit, | ||
onbefore, | ||
onafter, | ||
ondestroy, | ||
allowNoInit, | ||
logid, | ||
logtype } = {}) { | ||
oninit, | ||
onbefore, | ||
onafter, | ||
ondestroy, | ||
allowNoInit, | ||
logid, | ||
logtype | ||
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. not related to this PR about whitespace, but could we enforce trailing commas? makes things easier in git when another field is added and only have a 1 line change instead of 2. 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. I was trying to propose this for the docs, but it seems this lacks approvement: |
||
} = {}) { | ||
return new ActivityCollector(process.hrtime(), { | ||
oninit, | ||
onbefore, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,8 +18,8 @@ assert.throws(() => new AsyncResource('invalid_trigger_id', null), | |
/^RangeError: triggerAsyncId must be an unsigned integer$/); | ||
|
||
assert.strictEqual( | ||
new AsyncResource('default_trigger_id').triggerAsyncId(), | ||
async_hooks.executionAsyncId() | ||
new AsyncResource('default_trigger_id').triggerAsyncId(), | ||
async_hooks.executionAsyncId() | ||
); | ||
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. IMHO I'd prefer to put the assert.strictEqual(
new AsyncResource('default_trigger_id').triggerAsyncId(),
async_hooks.executionAsyncId()); But both of them are OK. |
||
|
||
// create first custom event 'alcazares' with triggerAsyncId derived | ||
|
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.
I guess this is very C++, but not my cup of tea.
If there are no objections I'd rather have
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.
Uhh I'd prefer the form in the PR, because imagine what that variant would look like if I were to add another option.
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.
Yeah, it's definatly at it's limit... Although at a later stage we could move the destructuring into the function's body...
Anyway, I'm not strongly opposed to current format.
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.
Improved readability slightly by adding a new line after
logtype
...