From e20af288d9de583ccef4fbbc3bbf836662946036 Mon Sep 17 00:00:00 2001 From: Federico Brigante Date: Fri, 25 Sep 2020 16:12:45 -0500 Subject: [PATCH 1/4] Promisify GitHub Factory --- package-lock.json | 30 ++++--- package.json | 6 +- src/github-factory.js | 129 +++++----------------------- src/index.js | 122 +++++++++++++------------- src/should-delete-fork.js | 176 ++++++++++++++++---------------------- 5 files changed, 177 insertions(+), 286 deletions(-) diff --git a/package-lock.json b/package-lock.json index d4559ec..8b4e6ce 100644 --- a/package-lock.json +++ b/package-lock.json @@ -331,6 +331,15 @@ "deprecation": "^2.3.1" } }, + "@octokit/plugin-throttling": { + "version": "3.3.0", + "resolved": "https://registry.npmjs.org/@octokit/plugin-throttling/-/plugin-throttling-3.3.0.tgz", + "integrity": "sha512-0hMtpqTAwAvo3XkNoaPQFqX2+csfbuR0bYpruHq3anrBrhQFPt9Gn01RVqqoDla/qBYmIIY2MRirxlaLO21Apg==", + "requires": { + "@octokit/types": "^5.0.0", + "bottleneck": "^2.15.3" + } + }, "@octokit/request": { "version": "5.4.9", "resolved": "https://registry.npmjs.org/@octokit/request/-/request-5.4.9.tgz", @@ -810,11 +819,6 @@ "integrity": "sha512-Z7tMw1ytTXt5jqMcOP+OQteU1VuNK9Y02uuJtKQ1Sv69jXQKKg5cibLwGJow8yzZP+eAc18EmLGPal0bp36rvQ==", "dev": true }, - "async": { - "version": "3.2.0", - "resolved": "https://registry.npmjs.org/async/-/async-3.2.0.tgz", - "integrity": "sha512-TR2mEZFVOj2pLStYxLht7TyfuRzaydfpxr3k9RpHIzMgw7A64dzsdqCxH1WJyQdoe8T10nDXd9wnEigmiuHIZw==" - }, "at-least-node": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/at-least-node/-/at-least-node-1.0.0.tgz", @@ -1017,6 +1021,11 @@ "integrity": "sha512-GkTiFpjFtUzU9CbMeJ5iazkCzGL3jrhzerzZIuqLABjbwRaFt33I9tUdSNryIptM+RxDet6OKm2WnLXzW51KsQ==", "dev": true }, + "bottleneck": { + "version": "2.19.5", + "resolved": "https://registry.npmjs.org/bottleneck/-/bottleneck-2.19.5.tgz", + "integrity": "sha512-VHiNCbI1lKdl44tGrhNfU3lup0Tj/ZBMJB5/2ZbNXRCPuRCO7ed2mgcK4r17y+KB2EfuYuRaVlwNbAeaWGSpbw==" + }, "boxen": { "version": "4.2.0", "resolved": "https://registry.npmjs.org/boxen/-/boxen-4.2.0.tgz", @@ -3545,7 +3554,8 @@ "inherits": { "version": "2.0.4", "resolved": "https://registry.npmjs.org/inherits/-/inherits-2.0.4.tgz", - "integrity": "sha512-k/vGaX4/Yla3WzyMCvTQOXYeIHvqOKtnqBduzTHpzpQZzAskKMhZ2K+EnBiSM9zGSoIFeMpXKxa4dYeZIQqewQ==" + "integrity": "sha512-k/vGaX4/Yla3WzyMCvTQOXYeIHvqOKtnqBduzTHpzpQZzAskKMhZ2K+EnBiSM9zGSoIFeMpXKxa4dYeZIQqewQ==", + "dev": true }, "ini": { "version": "1.3.5", @@ -5117,14 +5127,6 @@ "integrity": "sha1-nsYfeQSYdXB9aUFFlv2Qek1xHnM=", "dev": true }, - "queue": { - "version": "6.0.1", - "resolved": "https://registry.npmjs.org/queue/-/queue-6.0.1.tgz", - "integrity": "sha512-AJBQabRCCNr9ANq8v77RJEv73DPbn55cdTb+Giq4X0AVnNVZvMHlYp7XlQiN+1npCZj1DuSmaA2hYVUUDgxFDg==", - "requires": { - "inherits": "~2.0.3" - } - }, "quick-lru": { "version": "4.0.1", "resolved": "https://registry.npmjs.org/quick-lru/-/quick-lru-4.0.1.tgz", diff --git a/package.json b/package.json index 7a1dbbc..7a696ee 100644 --- a/package.json +++ b/package.json @@ -22,11 +22,11 @@ "npm": ">=6.10" }, "dependencies": { + "@octokit/plugin-throttling": "^3.3.0", "@octokit/rest": "^18.0.6", "async": "^3.2.0", "commander": "^6.1.0", "confirm-simple": "^1.0.3", - "queue": "^6.0.1", "single-line-log": "^1.1.2" }, "repository": { @@ -64,10 +64,6 @@ "no-var": "off", "prefer-rest-params": "off", "prefer-spread": "off", - "promise/always-return": "off", - "promise/catch-or-return": "off", - "promise/no-callback-in-promise": "off", - "promise/prefer-await-to-then": "off", "unicorn/explicit-length-check": "off", "unicorn/prevent-abbreviations": "off" } diff --git a/src/github-factory.js b/src/github-factory.js index 45c2f8b..21b0cc6 100644 --- a/src/github-factory.js +++ b/src/github-factory.js @@ -1,117 +1,34 @@ const {Octokit} = require('@octokit/rest'); -const queue = require('queue'); +const {throttling} = require('@octokit/plugin-throttling'); +const ThrottledOctokit = Octokit.plugin(throttling); -const rawGithubFactory = (token) => { +module.exports = (token) => { // Allow to inject a GitHub API instead of a token if (token.repos) { return token; } - return new Octokit({ + return new ThrottledOctokit({ auth: token, - version: '3.0.0' - }); -}; - -module.exports = (token) => { - const api = rawGithubFactory(token); - - const q = queue({ - concurrency: 1, - timeout: 2000 - }); - - // Transform a github function call into a queued function call - // to ensure that only one API call runs at a time - // https://developer.github.com/guides/best-practices-for-integrators/#dealing-with-abuse-rate-limits - const qd = (call) => { - if (typeof call !== 'function') { - throw new TypeError(`call should be a function: ${call}`); - } - - return (...arguments_) => - new Promise((resolve, reject) => { - q.push((callback) => { - let argumentsCallback = arguments_.pop(); - if (typeof argumentsCallback !== 'function') { - arguments_.push(argumentsCallback); - argumentsCallback = null; - } - - call.apply(null, arguments_).then( - (result) => { - callback(); - if (argumentsCallback) { - argumentsCallback(null, result); - } - - resolve(result); - }, - (error) => { - callback(); - if (argumentsCallback) { - argumentsCallback(error); - } - - reject(error); - } - ); - }); - q.start(); - }); - }; - - const makeResponseTransformer = (transform) => (call) => { - if (typeof call !== 'function') { - throw new TypeError(`call should be a function: ${call}`); - } - - return (...arguments_) => { - let argumentsCallback = arguments_.pop(); - if (typeof argumentsCallback !== 'function') { - arguments_.push(argumentsCallback); - argumentsCallback = null; - } - - return call.apply(null, arguments_).then( - (result) => { - result = transform(result); - if (argumentsCallback) { - argumentsCallback(null, result); - } - - return result; - }, - (error) => { - if (argumentsCallback) { - argumentsCallback(error); - } + version: '3.0.0', + throttle: { + onRateLimit: (retryAfter, options, octokit) => { + octokit.log.warn( + `Request quota exhausted for request ${options.method} ${options.url}` + ); + + if (options.request.retryCount === 0) { + // Only retries once + octokit.log.info(`Retrying after ${retryAfter} seconds!`); + return true; } - ); - }; - }; - - // Unwrap .data in the response - const nd = makeResponseTransformer(({data}) => data); - - // Add a warning about subtly invalid params - const hw = (f) => (...arguments_) => { - if (arguments_[0] && arguments_[0].url) { - throw new Error('Avoid passing url option'); - } - - return f.apply(null, arguments_); - }; - - const paginate = (f) => (...arguments_) => api.paginate(f, ...arguments_); - - return { - repos: { - compareCommits: hw(nd(qd(api.repos.compareCommits))), - delete: hw(nd(qd(api.repos.delete))), - get: hw(nd(qd(api.repos.get))), - list: hw(qd(paginate(api.repos.listForAuthenticatedUser))), - listBranches: hw(nd(qd(api.repos.listBranches))) + }, + onAbuseLimit: (retryAfter, options, octokit) => { + // Does not retry, only logs a warning + octokit.log.warn( + `Abuse detected for request ${options.method} ${options.url}` + ); + } } - }; + }); }; diff --git a/src/index.js b/src/index.js index 385cb12..c0ed479 100644 --- a/src/index.js +++ b/src/index.js @@ -20,7 +20,7 @@ const api = (token, options, callback) => { }); }; -api.get = (token, options, getCallback) => { +api.get = async (token, options, getCallback) => { if (!getCallback) { getCallback = options; options = {}; @@ -32,77 +32,77 @@ api.get = (token, options, getCallback) => { const github = githubFactory(token); // Get all repositories - github.repos - .list({type: 'public'}) - .then((repos) => { - // Keep only forks - let forks = repos.filter(({fork}) => fork); - - // Keep only forks owned by a specified user - if (options.user) { - forks = forks.filter(({owner}) => owner.login === options.user); - } + try { + const repos = await github.paginate(github.repos.listForAuthenticatedUser, { + type: 'public' + }); + // Keep only forks + let forks = repos.filter(({fork}) => fork); + + // Keep only forks owned by a specified user + if (options.user) { + forks = forks.filter(({owner}) => owner.login === options.user); + } - let countDoneForks = 0; + let countDoneForks = 0; + options.progress({ + countInspected: 0, + totalToInspect: forks.length + }); + const forkDone = (fork) => { + countDoneForks += 1; options.progress({ - countInspected: 0, + countInspected: countDoneForks, + lastInspected: fork.name, totalToInspect: forks.length }); - const forkDone = (fork) => { - countDoneForks += 1; - options.progress({ - countInspected: countDoneForks, - lastInspected: fork.name, - totalToInspect: forks.length - }); - }; - - // Keep only useless forks - async.filter( - forks, - (fork, filterCallback) => { - shouldDeleteFork(github, fork, (error, result) => { - if (error) { - options.warnings( - `Failed to inspect ${fork.name}, skipping`, - error - ); - result = false; - } - - forkDone(fork); - filterCallback(null, result); - }); - }, - (error, forksToDelete) => { + }; + + // Keep only useless forks + async.filter( + forks, + (fork, filterCallback) => { + shouldDeleteFork(github, fork, (error, result) => { if (error) { - return getCallback(error); + options.warnings(`Failed to inspect ${fork.name}, skipping`, error); + result = false; } - // Map to our simple objects - const response = forksToDelete.map((fork) => ({ - owner: fork.owner.login, - repo: fork.name, - url: fork.html_url - })); - getCallback(null, response); + forkDone(fork); + filterCallback(null, result); + }); + }, + (error, forksToDelete) => { + if (error) { + return getCallback(error); } - ); - }) - .then(null, (error) => { - getCallback(error); - }); + + // Map to our simple objects + const response = forksToDelete.map((fork) => ({ + owner: fork.owner.login, + repo: fork.name, + url: fork.html_url + })); + getCallback(null, response); + } + ); + } catch (error) { + getCallback(error); + } }; -api.remove = (token, repos, removeCallback) => { +api.remove = async (token, repos, removeCallback) => { const github = githubFactory(token); - async.each( - repos, - (repo, callback) => { - github.repos.delete({owner: repo.owner, repo: repo.repo}, callback); - }, - removeCallback - ); + try { + await Promise.all( + repos.map((repo) => + github.repos.delete({owner: repo.owner, repo: repo.repo}) + ) + ); + removeCallback(); + } catch (error) { + removeCallback(error); + } }; module.exports = api; diff --git a/src/should-delete-fork.js b/src/should-delete-fork.js index 01a42ca..11929ca 100644 --- a/src/should-delete-fork.js +++ b/src/should-delete-fork.js @@ -20,30 +20,26 @@ const branchIsUseful = (github, repo, branch, parentBranches, callback) => { }); async.some( branchesToCheck, - (candidate, someCallback) => { - github.repos.compareCommits( - { + async (candidate) => { + try { + const {data: diff} = await github.repos.compareCommits({ owner: repo.parent.owner.login, repo: repo.parent.name, base: branch.commit.sha, head: candidate.name - }, - (error, diff) => { - if (error) { - // If diff can't be found, means our commit is not on this - // parent branch candidate - if (error.status === 404) { - return someCallback(null, false); - } - - return someCallback(error); - } - - // If parent is not behind us, this parent candidate - // branch contains our branch - someCallback(null, !diff.behind_by); + }); + // If parent is not behind us, this parent candidate + // branch contains our branch + return !diff.behind_by; + } catch (error) { + // If diff can't be found, means our commit is not on this + // parent branch candidate + if (error.status === 404) { + return false; } - ); + + throw error; + } }, (error, parentContainsOurCommit) => { if (error) { @@ -55,94 +51,74 @@ const branchIsUseful = (github, repo, branch, parentBranches, callback) => { ); }; -module.exports = (github, fork, shouldDeleteCallback) => { - async.waterfall( - [ - // Grab the repository information - (callback) => { - github.repos.get({owner: fork.owner.login, repo: fork.name}, callback); - }, +module.exports = async (github, fork, shouldDeleteCallback) => { + try { + // Grab the repository information + const {data: repo} = await github.repos.get({ + owner: fork.owner.login, + repo: fork.name + }); - // Grab all branches - (repo, callback) => { - github.repos.listBranches( - { - owner: repo.owner.login, - repo: repo.name, - per_page: 100 - }, - (error, branches) => { - if (error) { - return callback(error); - } + // Grab all branches + const {data: branches} = await github.repos.listBranches({ + owner: repo.owner.login, + repo: repo.name, + per_page: 100 + }); - if (branches.length === 100) { - // There are too many branches, - // dealing with pagination is not supported - shouldDeleteCallback(null, false); - return; - } + if (branches.length === 100) { + // There are too many branches, + // dealing with pagination is not supported + shouldDeleteCallback(null, false); + return; + } - callback(null, repo, branches); - } - ); - }, + // Grab all parent repository branches + let parentbranches; + try { + ({data: parentbranches} = await github.repos.listBranches({ + owner: repo.parent.owner.login, + repo: repo.parent.name, + per_page: 100 + })); + } catch (error) { + // If parent repository was deleted, need to preserve the fork + if (error && error.status === 404) { + shouldDeleteCallback(null, false); + return; + } + + throw error; + } + + // Compare if for each local branch is useless + if (branches.length > parentbranches.length) { + // Quick common case + shouldDeleteCallback(null, false); + return; + } - // Grab all parent repository branches - (repo, branches, callback) => { - github.repos.listBranches( - { - owner: repo.parent.owner.login, - repo: repo.parent.name, - per_page: 100 - }, - (error, parentbranches) => { - // If parent repository was deleted, need to preserve the fork - if (error && error.status == 404) { - shouldDeleteCallback(null, false); - return; + async.some( + branches, + (branch, someCallback) => { + branchIsUseful( + github, + repo, + branch, + parentbranches, + (error, useful) => { + if (error) { + return someCallback(error); } - callback(error, repo, branches, parentbranches); + someCallback(null, useful); } ); }, - - // Compare if for each local branch is useless - (repo, branches, parentbranches, callback) => { - // Quick common case - if (branches.length > parentbranches.length) { - callback(null, true); - return; - } - - async.some( - branches, - (branch, someCallback) => { - branchIsUseful( - github, - repo, - branch, - parentbranches, - (error, useful) => { - if (error) { - return someCallback(error); - } - - someCallback(null, useful); - } - ); - }, - callback - ); - } - ], - (error, someBranchesUseful) => { - if (error) { - shouldDeleteCallback(error); - } else { - shouldDeleteCallback(null, !someBranchesUseful); - } - } - ); + (error, someBranchesUseful) => + shouldDeleteCallback(error, !someBranchesUseful) + ); + } catch (error) { + shouldDeleteCallback(error); + } }; From 27ee0c0661847279b2bef311823f46e82879f68d Mon Sep 17 00:00:00 2001 From: Federico Brigante Date: Fri, 25 Sep 2020 17:00:17 -0500 Subject: [PATCH 2/4] Promisify `branchIsUseful` --- src/should-delete-fork.js | 85 ++++++++++++++++----------------------- 1 file changed, 34 insertions(+), 51 deletions(-) diff --git a/src/should-delete-fork.js b/src/should-delete-fork.js index 11929ca..06be67e 100644 --- a/src/should-delete-fork.js +++ b/src/should-delete-fork.js @@ -1,15 +1,13 @@ 'use strict'; -const async = require('async'); - -const branchIsUseful = (github, repo, branch, parentBranches, callback) => { +const branchIsUseful = async (github, repo, branch, parentBranches) => { const someParentBranchAtThisSha = parentBranches.filter((candidate) => { return branch.commit.sha === candidate.commit.sha; }).length === 1; if (someParentBranchAtThisSha) { - return callback(null, false); + return false; } // Check if at least one parent branch contains this commit @@ -18,37 +16,32 @@ const branchIsUseful = (github, repo, branch, parentBranches, callback) => { const branchesToCheck = parentBranches.filter((b) => { return [branch.name, 'master'].includes(b.name); }); - async.some( - branchesToCheck, - async (candidate) => { - try { - const {data: diff} = await github.repos.compareCommits({ - owner: repo.parent.owner.login, - repo: repo.parent.name, - base: branch.commit.sha, - head: candidate.name - }); - // If parent is not behind us, this parent candidate - // branch contains our branch - return !diff.behind_by; - } catch (error) { - // If diff can't be found, means our commit is not on this - // parent branch candidate - if (error.status === 404) { - return false; - } - - throw error; + for (const candidate of branchesToCheck) { + try { + // eslint-disable-next-line no-await-in-loop + const {data: diff} = await github.repos.compareCommits({ + owner: repo.parent.owner.login, + repo: repo.parent.name, + base: branch.commit.sha, + head: candidate.name + }); + // If parent is not behind us, this parent candidate + // branch contains our branch + if (diff.behind_by) { + return true; } - }, - (error, parentContainsOurCommit) => { - if (error) { - callback(error); - } else { - callback(null, !parentContainsOurCommit); + } catch (error) { + // If diff can't be found, means our commit is not on this + // parent branch candidate + if (error.status === 404) { + continue; } + + throw error; } - ); + } + + return false; }; module.exports = async (github, fork, shouldDeleteCallback) => { @@ -98,26 +91,16 @@ module.exports = async (github, fork, shouldDeleteCallback) => { return; } - async.some( - branches, - (branch, someCallback) => { - branchIsUseful( - github, - repo, - branch, - parentbranches, - (error, useful) => { - if (error) { - return someCallback(error); - } + for (const branch of branches) { + // eslint-disable-next-line no-await-in-loop + const useful = await branchIsUseful(github, repo, branch, parentbranches); + if (useful) { + shouldDeleteCallback(null, false); + return; + } + } - someCallback(null, useful); - } - ); - }, - (error, someBranchesUseful) => - shouldDeleteCallback(error, !someBranchesUseful) - ); + shouldDeleteCallback(null, true); } catch (error) { shouldDeleteCallback(error); } From ed0044e3f659f63f05a14fb26ad81a6f441af223 Mon Sep 17 00:00:00 2001 From: Federico Brigante Date: Fri, 25 Sep 2020 17:24:36 -0500 Subject: [PATCH 3/4] Promisify `shouldDeleteFork` + drop `async` dependency --- package.json | 1 - src/index.js | 44 +++++++----------- src/should-delete-fork.js | 96 ++++++++++++++++++--------------------- 3 files changed, 61 insertions(+), 80 deletions(-) diff --git a/package.json b/package.json index 7a696ee..880b3b7 100644 --- a/package.json +++ b/package.json @@ -24,7 +24,6 @@ "dependencies": { "@octokit/plugin-throttling": "^3.3.0", "@octokit/rest": "^18.0.6", - "async": "^3.2.0", "commander": "^6.1.0", "confirm-simple": "^1.0.3", "single-line-log": "^1.1.2" diff --git a/src/index.js b/src/index.js index c0ed479..34e15dc 100644 --- a/src/index.js +++ b/src/index.js @@ -1,7 +1,5 @@ 'use strict'; -const async = require('async'); - const githubFactory = require('./github-factory'); const shouldDeleteFork = require('./should-delete-fork'); @@ -59,33 +57,25 @@ api.get = async (token, options, getCallback) => { }; // Keep only useless forks - async.filter( - forks, - (fork, filterCallback) => { - shouldDeleteFork(github, fork, (error, result) => { - if (error) { - options.warnings(`Failed to inspect ${fork.name}, skipping`, error); - result = false; - } - - forkDone(fork); - filterCallback(null, result); - }); - }, - (error, forksToDelete) => { - if (error) { - return getCallback(error); + const processedForks = forks.map(async (fork) => { + try { + if (await shouldDeleteFork(github, fork)) { + // Map to our simple objects + return { + owner: fork.owner.login, + repo: fork.name, + url: fork.html_url + }; } - - // Map to our simple objects - const response = forksToDelete.map((fork) => ({ - owner: fork.owner.login, - repo: fork.name, - url: fork.html_url - })); - getCallback(null, response); + } catch (error) { + options.warnings(`Failed to inspect ${fork.name}, skipping`, error); + } finally { + forkDone(fork); } - ); + }); + + const forksToDelete = await Promise.all(processedForks); + getCallback(null, forksToDelete.filter(Boolean)); } catch (error) { getCallback(error); } diff --git a/src/should-delete-fork.js b/src/should-delete-fork.js index 06be67e..16c0049 100644 --- a/src/should-delete-fork.js +++ b/src/should-delete-fork.js @@ -44,64 +44,56 @@ const branchIsUseful = async (github, repo, branch, parentBranches) => { return false; }; -module.exports = async (github, fork, shouldDeleteCallback) => { - try { - // Grab the repository information - const {data: repo} = await github.repos.get({ - owner: fork.owner.login, - repo: fork.name - }); - - // Grab all branches - const {data: branches} = await github.repos.listBranches({ - owner: repo.owner.login, - repo: repo.name, - per_page: 100 - }); +module.exports = async (github, fork) => { + // Grab the repository information + const {data: repo} = await github.repos.get({ + owner: fork.owner.login, + repo: fork.name + }); - if (branches.length === 100) { - // There are too many branches, - // dealing with pagination is not supported - shouldDeleteCallback(null, false); - return; - } + // Grab all branches + const {data: branches} = await github.repos.listBranches({ + owner: repo.owner.login, + repo: repo.name, + per_page: 100 + }); - // Grab all parent repository branches - let parentbranches; - try { - ({data: parentbranches} = await github.repos.listBranches({ - owner: repo.parent.owner.login, - repo: repo.parent.name, - per_page: 100 - })); - } catch (error) { - // If parent repository was deleted, need to preserve the fork - if (error && error.status === 404) { - shouldDeleteCallback(null, false); - return; - } + if (branches.length === 100) { + // There are too many branches, + // dealing with pagination is not supported + return false; + } - throw error; + // Grab all parent repository branches + let parentbranches; + try { + ({data: parentbranches} = await github.repos.listBranches({ + owner: repo.parent.owner.login, + repo: repo.parent.name, + per_page: 100 + })); + } catch (error) { + // If parent repository was deleted, need to preserve the fork + if (error && error.status === 404) { + return false; } - // Compare if for each local branch is useless - if (branches.length > parentbranches.length) { - // Quick common case - shouldDeleteCallback(null, false); - return; - } + throw error; + } - for (const branch of branches) { - // eslint-disable-next-line no-await-in-loop - const useful = await branchIsUseful(github, repo, branch, parentbranches); - if (useful) { - shouldDeleteCallback(null, false); - return; - } - } + // Compare if for each local branch is useless + if (branches.length > parentbranches.length) { + // Quick common case + return false; + } - shouldDeleteCallback(null, true); - } catch (error) { - shouldDeleteCallback(error); + for (const branch of branches) { + // eslint-disable-next-line no-await-in-loop + const useful = await branchIsUseful(github, repo, branch, parentbranches); + if (useful) { + return false; + } } + + return true; }; From ee0f470d44a1d6880e9b3baa2139868057e211f2 Mon Sep 17 00:00:00 2001 From: Federico Brigante Date: Sat, 26 Sep 2020 15:26:34 -0500 Subject: [PATCH 4/4] Preserve initial callback-only API --- src/index.js | 121 +++++++++++++++++++++++++++------------------------ 1 file changed, 63 insertions(+), 58 deletions(-) diff --git a/src/index.js b/src/index.js index 34e15dc..c230a1e 100644 --- a/src/index.js +++ b/src/index.js @@ -18,81 +18,86 @@ const api = (token, options, callback) => { }); }; -api.get = async (token, options, getCallback) => { - if (!getCallback) { - getCallback = options; - options = {}; - } - +const _apiGet = async (token, options = {}) => { options.progress = options.progress || (() => {}); options.warnings = options.warnings || (() => {}); const github = githubFactory(token); // Get all repositories - try { - const repos = await github.paginate(github.repos.listForAuthenticatedUser, { - type: 'public' - }); - // Keep only forks - let forks = repos.filter(({fork}) => fork); + const repos = await github.paginate(github.repos.listForAuthenticatedUser, { + type: 'public' + }); + // Keep only forks + let forks = repos.filter(({fork}) => fork); - // Keep only forks owned by a specified user - if (options.user) { - forks = forks.filter(({owner}) => owner.login === options.user); - } + // Keep only forks owned by a specified user + if (options.user) { + forks = forks.filter(({owner}) => owner.login === options.user); + } - let countDoneForks = 0; + let countDoneForks = 0; + options.progress({ + countInspected: 0, + totalToInspect: forks.length + }); + const forkDone = (fork) => { + countDoneForks += 1; options.progress({ - countInspected: 0, + countInspected: countDoneForks, + lastInspected: fork.name, totalToInspect: forks.length }); - const forkDone = (fork) => { - countDoneForks += 1; - options.progress({ - countInspected: countDoneForks, - lastInspected: fork.name, - totalToInspect: forks.length - }); - }; - - // Keep only useless forks - const processedForks = forks.map(async (fork) => { - try { - if (await shouldDeleteFork(github, fork)) { - // Map to our simple objects - return { - owner: fork.owner.login, - repo: fork.name, - url: fork.html_url - }; - } - } catch (error) { - options.warnings(`Failed to inspect ${fork.name}, skipping`, error); - } finally { - forkDone(fork); + }; + + // Keep only useless forks + const processedForks = forks.map(async (fork) => { + try { + if (await shouldDeleteFork(github, fork)) { + // Map to our simple objects + return { + owner: fork.owner.login, + repo: fork.name, + url: fork.html_url + }; } - }); + } catch (error) { + options.warnings(`Failed to inspect ${fork.name}, skipping`, error); + } finally { + forkDone(fork); + } + }); - const forksToDelete = await Promise.all(processedForks); - getCallback(null, forksToDelete.filter(Boolean)); - } catch (error) { - getCallback(error); - } + const forksToDelete = await Promise.all(processedForks); + return forksToDelete.filter(Boolean); }; -api.remove = async (token, repos, removeCallback) => { +const _apiRemove = async (token, repos) => { const github = githubFactory(token); - try { - await Promise.all( - repos.map((repo) => - github.repos.delete({owner: repo.owner, repo: repo.repo}) - ) - ); - removeCallback(); - } catch (error) { - removeCallback(error); + await Promise.all( + repos.map((repo) => + github.repos.delete({owner: repo.owner, repo: repo.repo}) + ) + ); +}; + +/* eslint-disable promise/catch-or-return, promise/prefer-await-to-then, promise/always-return, promise/no-callback-in-promise */ +api.remove = (token, repos, callback) => { + _apiRemove(token, repos).then((result) => { + callback(null, result); + }, callback); +}; + +api.get = (token, options, callback) => { + if (!callback) { + callback = options; + options = {}; } + + _apiGet(token, options).then((result) => { + callback(null, result); + }, callback); }; +/* eslint-enable promise/catch-or-return, promise/prefer-await-to-then, promise/always-return, promise/no-callback-in-promise */ module.exports = api;