Skip to content

Commit 3ebc96d

Browse files
authored
Merge pull request #112 from denis-sokolov/lint-further
Promisify internals
2 parents c4bc7e6 + ee0f470 commit 3ebc96d

File tree

5 files changed

+179
-319
lines changed

5 files changed

+179
-319
lines changed

package-lock.json

Lines changed: 16 additions & 14 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,10 @@
2222
"npm": ">=6.10"
2323
},
2424
"dependencies": {
25+
"@octokit/plugin-throttling": "^3.3.0",
2526
"@octokit/rest": "^18.0.6",
26-
"async": "^3.2.0",
2727
"commander": "^6.1.0",
2828
"confirm-simple": "^1.0.3",
29-
"queue": "^6.0.1",
3029
"single-line-log": "^1.1.2"
3130
},
3231
"repository": {
@@ -64,10 +63,6 @@
6463
"no-var": "off",
6564
"prefer-rest-params": "off",
6665
"prefer-spread": "off",
67-
"promise/always-return": "off",
68-
"promise/catch-or-return": "off",
69-
"promise/no-callback-in-promise": "off",
70-
"promise/prefer-await-to-then": "off",
7166
"unicorn/explicit-length-check": "off",
7267
"unicorn/prevent-abbreviations": "off"
7368
}

src/github-factory.js

Lines changed: 23 additions & 106 deletions
Original file line numberDiff line numberDiff line change
@@ -1,117 +1,34 @@
11
const {Octokit} = require('@octokit/rest');
2-
const queue = require('queue');
2+
const {throttling} = require('@octokit/plugin-throttling');
3+
const ThrottledOctokit = Octokit.plugin(throttling);
34

4-
const rawGithubFactory = (token) => {
5+
module.exports = (token) => {
56
// Allow to inject a GitHub API instead of a token
67
if (token.repos) {
78
return token;
89
}
910

10-
return new Octokit({
11+
return new ThrottledOctokit({
1112
auth: token,
12-
version: '3.0.0'
13-
});
14-
};
15-
16-
module.exports = (token) => {
17-
const api = rawGithubFactory(token);
18-
19-
const q = queue({
20-
concurrency: 1,
21-
timeout: 2000
22-
});
23-
24-
// Transform a github function call into a queued function call
25-
// to ensure that only one API call runs at a time
26-
// https://developer.github.com/guides/best-practices-for-integrators/#dealing-with-abuse-rate-limits
27-
const qd = (call) => {
28-
if (typeof call !== 'function') {
29-
throw new TypeError(`call should be a function: ${call}`);
30-
}
31-
32-
return (...arguments_) =>
33-
new Promise((resolve, reject) => {
34-
q.push((callback) => {
35-
let argumentsCallback = arguments_.pop();
36-
if (typeof argumentsCallback !== 'function') {
37-
arguments_.push(argumentsCallback);
38-
argumentsCallback = null;
39-
}
40-
41-
call.apply(null, arguments_).then(
42-
(result) => {
43-
callback();
44-
if (argumentsCallback) {
45-
argumentsCallback(null, result);
46-
}
47-
48-
resolve(result);
49-
},
50-
(error) => {
51-
callback();
52-
if (argumentsCallback) {
53-
argumentsCallback(error);
54-
}
55-
56-
reject(error);
57-
}
58-
);
59-
});
60-
q.start();
61-
});
62-
};
63-
64-
const makeResponseTransformer = (transform) => (call) => {
65-
if (typeof call !== 'function') {
66-
throw new TypeError(`call should be a function: ${call}`);
67-
}
68-
69-
return (...arguments_) => {
70-
let argumentsCallback = arguments_.pop();
71-
if (typeof argumentsCallback !== 'function') {
72-
arguments_.push(argumentsCallback);
73-
argumentsCallback = null;
74-
}
75-
76-
return call.apply(null, arguments_).then(
77-
(result) => {
78-
result = transform(result);
79-
if (argumentsCallback) {
80-
argumentsCallback(null, result);
81-
}
82-
83-
return result;
84-
},
85-
(error) => {
86-
if (argumentsCallback) {
87-
argumentsCallback(error);
88-
}
13+
version: '3.0.0',
14+
throttle: {
15+
onRateLimit: (retryAfter, options, octokit) => {
16+
octokit.log.warn(
17+
`Request quota exhausted for request ${options.method} ${options.url}`
18+
);
19+
20+
if (options.request.retryCount === 0) {
21+
// Only retries once
22+
octokit.log.info(`Retrying after ${retryAfter} seconds!`);
23+
return true;
8924
}
90-
);
91-
};
92-
};
93-
94-
// Unwrap .data in the response
95-
const nd = makeResponseTransformer(({data}) => data);
96-
97-
// Add a warning about subtly invalid params
98-
const hw = (f) => (...arguments_) => {
99-
if (arguments_[0] && arguments_[0].url) {
100-
throw new Error('Avoid passing url option');
101-
}
102-
103-
return f.apply(null, arguments_);
104-
};
105-
106-
const paginate = (f) => (...arguments_) => api.paginate(f, ...arguments_);
107-
108-
return {
109-
repos: {
110-
compareCommits: hw(nd(qd(api.repos.compareCommits))),
111-
delete: hw(nd(qd(api.repos.delete))),
112-
get: hw(nd(qd(api.repos.get))),
113-
list: hw(qd(paginate(api.repos.listForAuthenticatedUser))),
114-
listBranches: hw(nd(qd(api.repos.listBranches)))
25+
},
26+
onAbuseLimit: (retryAfter, options, octokit) => {
27+
// Does not retry, only logs a warning
28+
octokit.log.warn(
29+
`Abuse detected for request ${options.method} ${options.url}`
30+
);
31+
}
11532
}
116-
};
33+
});
11734
};

src/index.js

Lines changed: 68 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
'use strict';
22

3-
const async = require('async');
4-
53
const githubFactory = require('./github-factory');
64
const shouldDeleteFork = require('./should-delete-fork');
75

@@ -20,89 +18,86 @@ const api = (token, options, callback) => {
2018
});
2119
};
2220

23-
api.get = (token, options, getCallback) => {
24-
if (!getCallback) {
25-
getCallback = options;
26-
options = {};
27-
}
28-
21+
const _apiGet = async (token, options = {}) => {
2922
options.progress = options.progress || (() => {});
3023
options.warnings = options.warnings || (() => {});
3124

3225
const github = githubFactory(token);
3326

3427
// Get all repositories
35-
github.repos
36-
.list({type: 'public'})
37-
.then((repos) => {
38-
// Keep only forks
39-
let forks = repos.filter(({fork}) => fork);
40-
41-
// Keep only forks owned by a specified user
42-
if (options.user) {
43-
forks = forks.filter(({owner}) => owner.login === options.user);
44-
}
28+
const repos = await github.paginate(github.repos.listForAuthenticatedUser, {
29+
type: 'public'
30+
});
31+
// Keep only forks
32+
let forks = repos.filter(({fork}) => fork);
4533

46-
let countDoneForks = 0;
47-
options.progress({
48-
countInspected: 0,
49-
totalToInspect: forks.length
50-
});
51-
const forkDone = (fork) => {
52-
countDoneForks += 1;
53-
options.progress({
54-
countInspected: countDoneForks,
55-
lastInspected: fork.name,
56-
totalToInspect: forks.length
57-
});
58-
};
59-
60-
// Keep only useless forks
61-
async.filter(
62-
forks,
63-
(fork, filterCallback) => {
64-
shouldDeleteFork(github, fork, (error, result) => {
65-
if (error) {
66-
options.warnings(
67-
`Failed to inspect ${fork.name}, skipping`,
68-
error
69-
);
70-
result = false;
71-
}
72-
73-
forkDone(fork);
74-
filterCallback(null, result);
75-
});
76-
},
77-
(error, forksToDelete) => {
78-
if (error) {
79-
return getCallback(error);
80-
}
81-
82-
// Map to our simple objects
83-
const response = forksToDelete.map((fork) => ({
84-
owner: fork.owner.login,
85-
repo: fork.name,
86-
url: fork.html_url
87-
}));
88-
getCallback(null, response);
89-
}
90-
);
91-
})
92-
.then(null, (error) => {
93-
getCallback(error);
34+
// Keep only forks owned by a specified user
35+
if (options.user) {
36+
forks = forks.filter(({owner}) => owner.login === options.user);
37+
}
38+
39+
let countDoneForks = 0;
40+
options.progress({
41+
countInspected: 0,
42+
totalToInspect: forks.length
43+
});
44+
const forkDone = (fork) => {
45+
countDoneForks += 1;
46+
options.progress({
47+
countInspected: countDoneForks,
48+
lastInspected: fork.name,
49+
totalToInspect: forks.length
9450
});
51+
};
52+
53+
// Keep only useless forks
54+
const processedForks = forks.map(async (fork) => {
55+
try {
56+
if (await shouldDeleteFork(github, fork)) {
57+
// Map to our simple objects
58+
return {
59+
owner: fork.owner.login,
60+
repo: fork.name,
61+
url: fork.html_url
62+
};
63+
}
64+
} catch (error) {
65+
options.warnings(`Failed to inspect ${fork.name}, skipping`, error);
66+
} finally {
67+
forkDone(fork);
68+
}
69+
});
70+
71+
const forksToDelete = await Promise.all(processedForks);
72+
return forksToDelete.filter(Boolean);
9573
};
9674

97-
api.remove = (token, repos, removeCallback) => {
75+
const _apiRemove = async (token, repos) => {
9876
const github = githubFactory(token);
99-
async.each(
100-
repos,
101-
(repo, callback) => {
102-
github.repos.delete({owner: repo.owner, repo: repo.repo}, callback);
103-
},
104-
removeCallback
77+
await Promise.all(
78+
repos.map((repo) =>
79+
github.repos.delete({owner: repo.owner, repo: repo.repo})
80+
)
10581
);
10682
};
10783

84+
/* eslint-disable promise/catch-or-return, promise/prefer-await-to-then, promise/always-return, promise/no-callback-in-promise */
85+
api.remove = (token, repos, callback) => {
86+
_apiRemove(token, repos).then((result) => {
87+
callback(null, result);
88+
}, callback);
89+
};
90+
91+
api.get = (token, options, callback) => {
92+
if (!callback) {
93+
callback = options;
94+
options = {};
95+
}
96+
97+
_apiGet(token, options).then((result) => {
98+
callback(null, result);
99+
}, callback);
100+
};
101+
/* eslint-enable promise/catch-or-return, promise/prefer-await-to-then, promise/always-return, promise/no-callback-in-promise */
102+
108103
module.exports = api;

0 commit comments

Comments
 (0)