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.

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

Add fixer for no-skip-test rule #183

Merged
merged 8 commits into from
Dec 6, 2017
Merged

Conversation

florianb
Copy link
Contributor

Introduces fixes for the no-skip-test-rule as requested in #21.

This work is based on the implementation of the fix for the no-only-test-rule.

Florian Breisch added 3 commits November 27, 2017 14:17
Includes refactorings to reuse code introduced with making
`ino-only-test` fixable.
@florianb florianb changed the title No skip fix No-skip-test fix Nov 27, 2017
@florianb florianb changed the title No-skip-test fix WIP: No-skip-test fix Nov 30, 2017
This is necessary to enable node 4 compliance.
@florianb florianb changed the title WIP: No-skip-test fix No-skip-test fix Nov 30, 2017
@florianb
Copy link
Contributor Author

Ready to 🚢

@sindresorhus
Copy link
Member

sindresorhus commented Dec 2, 2017

Don't commit package-lock.json. How did you even get it? We have

package-lock=false

@sindresorhus sindresorhus changed the title No-skip-test fix Add fixer for no-skip-test rule Dec 2, 2017
util.js Outdated
while (source.charAt(dotPosition) !== '.') {
dotPosition -= 1;
}
let snippet = source.substring(dotPosition, range[1]);
Copy link
Member

Choose a reason for hiding this comment

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

Can you use slice instead of substring? I know it was like this already, but better for consistency. I always use slice over substring.

sindresorhus/eslint-plugin-unicorn#65

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Of course - i am happily copying (most of) your habits.. 😸

@florianb
Copy link
Contributor Author

florianb commented Dec 4, 2017

I really have no clue, how the package-lock was generated - i'm not using more commands than npm test. It's probably a good idea adding the package-lock to the .gitognore.

@florianb
Copy link
Contributor Author

florianb commented Dec 4, 2017

Aehm - why didn't xo alert on the use of substring? 🤔 got it.

Copy link
Contributor

@jfmengels jfmengels left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks @florianb :)

@sindresorhus sindresorhus merged commit cf1f140 into avajs:master Dec 6, 2017
@sindresorhus
Copy link
Member

Thank you @florianb :)

@florianb florianb deleted the no-skip-fix branch December 7, 2017 17:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants