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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Sep 28, 2020
Merged

Lint everything else #116

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
af8bedb
Don't skip a passing test
fregante Sep 28, 2020
7910b43
Enable most disabled linting rules
fregante Sep 25, 2020
427c7b9
Disable no-todo-test rule
denis-sokolov Sep 28, 2020
4d0eeb3
Minor refactor in test progress
denis-sokolov Sep 28, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 2 additions & 11 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,18 +47,9 @@
],
"prettier": true,
"rules": {
"camelcase": "off",
"ava/no-import-test-files": "off",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Until avajs/eslint-plugin-ava#311

"ava/no-invalid-end": "off",
"ava/no-skip-test": "off",
"ava/no-statement-after-end": "off",
"eqeqeq": "off",
"no-useless-call": "off",
"no-var": "off",
"prefer-rest-params": "off",
"prefer-spread": "off",
"unicorn/explicit-length-check": "off",
"unicorn/prevent-abbreviations": "off"
"ava/no-todo-test": "off",
"camelcase": "off"
Copy link
Collaborator Author

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

}
}
}
2 changes: 1 addition & 1 deletion src/bin.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ try {
return abort(error);
}

if (!repos.length) {
if (repos.length === 0) {
console.log('No useless repositories found.');
process.exit(0);
}
Expand Down
2 changes: 1 addition & 1 deletion test/helpers/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ exports.mock = (responses) => {
const methodsWithoutDataField = ['listForAuthenticatedUser'];
const data =
typeof responsePassed === 'function'
? responsePassed.call(null, callData)
? responsePassed(callData)
: responsePassed;
const response = methodsWithoutDataField.includes(name) ? data : {data};
return new Promise((resolve) => {
Expand Down
5 changes: 2 additions & 3 deletions test/progress.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ test.cb('should report progress', (t) => {
mock.present,
{
progress(info) {
if (progressCallCount === 0) {
progressCallCount += 1;
if (progressCallCount === 1) {
t.is(info.countInspected, 0);
t.is(info.totalToInspect, 1);
} else {
Expand All @@ -23,8 +24,6 @@ test.cb('should report progress', (t) => {
t.is(info.lastInspected, 'fork1');
t.end();
}

progressCallCount += 1;
}
},
(error) => {
Expand Down
64 changes: 33 additions & 31 deletions test/simple.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Copy link
Owner

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.

const mock = lib.mock({
listBranches(arguments_) {
if (arguments_.user === lib.USER.login) {
test.cb(
'should delete a fork that has more branches, but all at upstream branch tips',
Comment on lines +76 to +77
Copy link
Owner

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.

Copy link
Collaborator Author

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.

Copy link
Owner

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

(t) => {
const mock = lib.mock({
listBranches(arguments_) {
if (arguments_.user === lib.USER.login) {
return [
{name: 'master', commit: lib.COMMIT_A},
{name: 'foo', commit: lib.COMMIT_A},
{name: 'bar', commit: lib.COMMIT_B}
];
}

return [
{name: 'master', commit: lib.COMMIT_A},
{name: 'foo', commit: lib.COMMIT_A},
{name: 'bar', commit: lib.COMMIT_B}
{name: 'baz', commit: lib.COMMIT_B}
];
}

return [
{name: 'master', commit: lib.COMMIT_A},
{name: 'baz', commit: lib.COMMIT_B}
];
},
delete: true
});
},
delete: true
});

removeGithubForks(mock.present, (error) => {
if (error) return t.fail(error);
lib.check(t, mock.calls(), [
['listForAuthenticatedUser', {type: 'public'}],
['get', {owner: lib.USER.login, repo: 'fork1'}],
['listBranches', {owner: lib.USER.login, repo: 'fork1', per_page: 100}],
[
'listBranches',
{owner: lib.AUTHOR.login, repo: 'upstream-lib', per_page: 100}
],
['delete', {owner: lib.USER.login, repo: 'fork1', url: undefined}]
]);
t.end();
});
});
removeGithubForks(mock.present, (error) => {
if (error) return t.fail(error);
lib.check(t, mock.calls(), [
['listForAuthenticatedUser', {type: 'public'}],
['get', {owner: lib.USER.login, repo: 'fork1'}],
['listBranches', {owner: lib.USER.login, repo: 'fork1', per_page: 100}],
[
'listBranches',
{owner: lib.AUTHOR.login, repo: 'upstream-lib', per_page: 100}
],
['delete', {owner: lib.USER.login, repo: 'fork1', url: undefined}]
]);
t.end();
});
}
);

test.cb(
'should delete a fork that has all branches at upstream branch tips',
Expand Down