-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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: refactor indexOf uses to es6 equivalents #13853
Conversation
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.
Nice work. Please feel free to also give us any feedback about the onboarding process today.
#goodnessSquad
test/pummel/test-dtrace-jsstack.js
Outdated
continue; | ||
|
||
const frame = line.substr(line.indexOf(sentinel) + sentinel.length); | ||
const top = frames.shift(); | ||
|
||
assert.strictEqual(frame.indexOf(top), 0, | ||
assert.ok(frame.startsWith(top) === true, |
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.
The === true
part can be left out here.
test/pummel/test-dtrace-jsstack.js
Outdated
continue; | ||
|
||
const frame = line.substr(line.indexOf(sentinel) + sentinel.length); | ||
const top = frames.shift(); | ||
|
||
assert.strictEqual(frame.indexOf(top), 0, | ||
assert.ok(frame.startsWith(top) === true, | ||
`unexpected frame where ${top} was expected`); |
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.
It is now linter issue:
96:24 error Expected indentation of 14 spaces but found 23 indent-legacy
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.
LGTM with nits addressed.
refactor indexOf uses to es6 where applicable Refs: nodejs#12586
I've fixed the rejects, how do i make the CI run again? |
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.
LGTM. CI failure on linux-fips is unrelated.
I think these changes were already covered in #13852 (amongst others) 😕 . Thank you for your contribution, I am sorry these changes did not make it in. Keep up the good work! |
refactor indexOf uses to es6 where applicable
Refs: #12586
#goodnessSquad