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

Clean up some helpers #1117

Merged
merged 8 commits into from
Oct 6, 2017
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
2 changes: 1 addition & 1 deletion lib/github-auth.js
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ function sendTokenToAllServers(token) {
strictSSL: false,
};
request(options, function(err, res, body) {
if (err != null) { return reject('Posting the GitHub user token failed: ' + err.stack); }
if (err != null) { return reject(err); }
Copy link
Member

Choose a reason for hiding this comment

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

Could do return reject(new Error('Posting the GitHub user token failed: ' + err.message)) or something similar. Up to you.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yea… I thought about that. This is called in a loop, and the calling code already has something really similar to that. It feels redundant to prefix it twice.

resolve();
});
});
Expand Down
8 changes: 1 addition & 7 deletions lib/github-provider.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ const {

// GitHub commits since integration.
function mapGithubCommitsSince(camp, githubApiUrl, githubAuth) {
camp.route(/^\/github\/commits-since\/([^\/]+)\/([^\/]+)\/([^\/]+)\.(svg|png|gif|jpg|json)$/,
camp.route(/^\/github\/commits-since\/([^/]+)\/([^/]+)\/([^/]+)\.(svg|png|gif|jpg|json)$/,
cache(function (data, match, sendBadge, request) {
const user = match[1]; // eg, SubtitleEdit
const repo = match[2]; // eg, subtitleedit
Expand Down Expand Up @@ -54,23 +54,17 @@ function mapGithubCommitsSince(camp, githubApiUrl, githubAuth) {
try {
const data = JSON.parse(buffer);
setCommitsSinceBadge(user, repo, data.tag_name);
return;
} catch (e) {
badgeData.text[1] = 'invalid';
sendBadge(format, badgeData);
return;
}
});
} else {
setCommitsSinceBadge(user, repo, version);
}

}));
}




module.exports = {
mapGithubCommitsSince
};
2 changes: 1 addition & 1 deletion lib/lru-cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ function getHeapSize() {
}
}
function computeHeapSize() {
return heapSize = process.memoryUsage().heapTotal;
return (heapSize = process.memoryUsage().heapTotal);
}

module.exports = Cache;
8 changes: 2 additions & 6 deletions lib/nexus-version.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,8 @@
'use strict';

function isSnapshotVersion(version) {
const pattern = /(\d+\.)*\d\-SNAPSHOT/;
if (version) {
return version.match(pattern);
} else {
return false;
}
const pattern = /(\d+\.)*\d-SNAPSHOT/;
return version && version.match(pattern);
Copy link
Member

Choose a reason for hiding this comment

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

Nice.

}

module.exports = {
Expand Down
2 changes: 1 addition & 1 deletion lib/npm-provider.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ const { metric } = require('./text-formatters');

// npm downloads count
function mapNpmDownloads(camp, urlComponent, apiUriComponent) {
camp.route(new RegExp('^\/npm\/' + urlComponent + '\/(.*)\.(svg|png|gif|jpg|json)$'),
camp.route(new RegExp('^/npm/' + urlComponent + '/(.*)\\.(svg|png|gif|jpg|json)$'),
cache(function(data, match, sendBadge, request) {
const pkg = encodeURIComponent(match[1]); // eg, "express" or "@user/express"
const format = match[2];
Expand Down
2 changes: 1 addition & 1 deletion lib/php-version.js
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ function numberedVersionData(version) {
// Try to convert to a list of numbers.
function toNum (s) {
let n = +s;
if (n !== n) { // If n is NaN…
if (Number.isNaN(n)) {
n = 0xffffffff;
}
return n;
Expand Down
4 changes: 2 additions & 2 deletions lib/regular-update.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const request = require('request');

// Map from URL to { timestamp: last fetch time, interval: in milliseconds,
// data: data }.
let regularUpdateCache = Object.create(null);
let regularUpdateCache = new Map();
Copy link
Member

Choose a reason for hiding this comment

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

All regularUpdateCache[url] need to be changed to either regularUpdateCache.get(url) or regularUpdateCache.set(url, value) in order to use Map

Copy link
Member Author

Choose a reason for hiding this comment

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

We have no unit test or service test coverage on regularUpdate. I started to write unit tests but think the time is better spent integrating a library like https://github.com/ptarjan/node-cache or https://github.com/mpneuried/nodecache, which have their own tests, than maintaining this code.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm going to reset these changes.


// url: a string, scraper: a function that takes string data at that URL.
// interval: number in milliseconds.
Expand Down Expand Up @@ -33,7 +33,7 @@ function regularUpdate(url, interval, scraper, cb) {
}

function clearRegularUpdateCache() {
regularUpdateCache = Object.create(null);
regularUpdateCache = new Map();
}

module.exports = {
Expand Down
20 changes: 10 additions & 10 deletions lib/result-sender.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,22 @@
'use strict';

const stream = require('stream');
const log = require('./log');
const svg2img = require('./svg-to-img');

function streamFromString(str) {
const newStream = new stream.Readable();
newStream._read = () => { newStream.push(str); newStream.push(null); };
return newStream;
}

function makeSend(format, askres, end) {
if (format === 'svg') {
return function(res) { sendSVG(res, askres, end); };
return res => sendSVG(res, askres, end);
} else if (format === 'json') {
return function(res) { sendJSON(res, askres, end); };
return res => sendJSON(res, askres, end);
} else {
return function(res) { sendOther(format, res, askres, end); };
return res => sendOther(format, res, askres, end);
}
}

Expand Down Expand Up @@ -37,13 +44,6 @@ function sendJSON(res, askres, end) {
end(null, {template: streamFromString(res)});
}

const stream = require('stream');
function streamFromString(str) {
const newStream = new stream.Readable();
newStream._read = function() { newStream.push(str); newStream.push(null); };
return newStream;
}

module.exports = {
makeSend
};
10 changes: 5 additions & 5 deletions lib/suggest.js
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ function githubLicense (user, repo) {
let licenseFilename;
// Crawl each file in the root directory
for (let i = 0; i < treeArray.length; i++) {
if (treeArray[i].type != 'blob') {
if (treeArray[i].type !== 'blob') {
continue;
}
if (treeArray[i].path.match(/(LICENSE|COPYING|COPYRIGHT).*/i)) {
Expand Down Expand Up @@ -179,13 +179,13 @@ function githubLicense (user, repo) {
resolve(null);
return;
}
} catch(e) { reject(e); return; }
} catch(e) { reject(e); }
});
} catch(e) { reject(e); return; }
} catch(e) { reject(e); }
});
} catch(e) { reject(e); return; }
} catch(e) { reject(e); }
});
} catch(e) { reject(e); return; }
} catch(e) { reject(e); }
});
});
}
Expand Down
4 changes: 2 additions & 2 deletions lib/version.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ function latestDottedVersion(versions) {
// Take string versions.
// -1 if v1 < v2, 1 if v1 > v2, 0 otherwise.
function compareDottedVersion(v1, v2) {
const parts1 = /([0-9\.]+)(.*)$/.exec(v1);
const parts2 = /([0-9\.]+)(.*)$/.exec(v2);
const parts1 = /([0-9.]+)(.*)$/.exec(v1);
const parts2 = /([0-9.]+)(.*)$/.exec(v2);
if (parts1 != null && parts2 != null) {
const numbers1 = parts1[1];
const numbers2 = parts2[1];
Expand Down
4 changes: 2 additions & 2 deletions service-tests/bower.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,14 @@ t.create('pre version. eg. bower|v0.2.5-alpha-rc-pre')
.get('/vpre/bootstrap.json')
.expectJSONTypes(Joi.object().keys({
name: Joi.equal('bower'),
value: Joi.string().regex(/^v\d+(\.\d+)?(\.\d+)?(\-?\w)+?$/)
value: Joi.string().regex(/^v\d+(\.\d+)?(\.\d+)?(-?\w)+?$/)
}));

t.create('custom label for pre version. eg. pre verison|v0.2.5-alpha-rc-pre')
.get('/vpre/bootstrap.json?label="pre verison"')
.expectJSONTypes(Joi.object().keys({
name: Joi.equal('pre verison'),
value: Joi.string().regex(/^v\d+(\.\d+)?(\.\d+)?(\-?\w)+?$/)
value: Joi.string().regex(/^v\d+(\.\d+)?(\.\d+)?(-?\w)+?$/)
}));


Expand Down
4 changes: 2 additions & 2 deletions service-tests/codecov.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ t.create('gets coverage status')
.get('/c/github/codecov/example-python.json')
.expectJSONTypes(Joi.object().keys({
name: 'coverage',
value: Joi.string().regex(/^[0-9]+\%$/),
value: Joi.string().regex(/^[0-9]+%$/),
}));

t.create('gets coverate status for branch')
.get('/c/github/codecov/example-python/master.json')
.expectJSONTypes({
name: 'coverage',
value: Joi.string().regex(/^[0-9]+\%$/),
value: Joi.string().regex(/^[0-9]+%$/),
});