-
Notifications
You must be signed in to change notification settings - Fork 21
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
Lint everything else #116
Lint everything else #116
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.
There was a test being skipped, but it seems to work. Feel free to revert the last commit if the test isn't ready.
@@ -47,18 +47,9 @@ | |||
], | |||
"prettier": true, | |||
"rules": { | |||
"camelcase": "off", | |||
"ava/no-import-test-files": "off", |
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.
"prefer-spread": "off", | ||
"unicorn/explicit-length-check": "off", | ||
"unicorn/prevent-abbreviations": "off" | ||
"camelcase": "off" |
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.
Due to GitHub’s API
16477d7
to
427c7b9
Compare
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.
Thanks!
@@ -73,41 +73,43 @@ test.cb('should not delete a fork that has more branches', (t) => { | |||
}); | |||
}); | |||
|
|||
// Currently our logic does not delete this fork | |||
test.skip('should delete a fork that has more branches, but all at upstream branch tips', (t) => { |
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.
Hecks knows, I don’t remember why it works. Perhaps I had added the functionality afterwards and forgot to enable the test? Weird.
test.cb( | ||
'should delete a fork that has more branches, but all at upstream branch tips', |
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.
eslint strangely formats this. If the test.cb
part is anything other than test.skip
, it puts arguments on separate lines. If it’s test.skip
, then both arguments are on one line. That’s with ava/no-skip-test disabled.
At first I thought it’s due to the line length, but no, even in the PR diff the shorter version is wrapped, and the longer isn’t. And, also, test.ski
is wrapped, and test.skipp
is wrapped. Weird.
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.
That’s Prettier. Yeah I don’t like Prettier’s wrapping either 😉
XO doesn’t care about line length as long as there’s no more than one ;
per line.
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.
You’re right that it’s prettier! prettier/prettier#9285
Manual lint of the last few rules