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

Using generators internally + refactoring flow #23

Merged
merged 4 commits into from
Jul 10, 2015
Merged
Show file tree
Hide file tree
Changes from 3 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
7 changes: 5 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@

NODE ?= node
NODE_FLAGS ?= $(shell $(NODE) --v8-options | grep generators | cut -d ' ' -f 3)

BIN := ./node_modules/.bin
MOCHA := $(BIN)/mocha
MOCHA := $(BIN)/_mocha
ESLINT := $(BIN)/eslint

test: node_modules
@$(MOCHA)
@$(NODE) $(NODE_FLAGS) $(MOCHA)

node_modules: package.json
@npm install
Expand Down
137 changes: 0 additions & 137 deletions index.js

This file was deleted.

134 changes: 91 additions & 43 deletions lib/github.js
Original file line number Diff line number Diff line change
@@ -1,62 +1,110 @@
/* eslint-disable camelcase */

var base = 'https://api.github.com/';
/**
* Module dependencies.
*/

var debug = require('debug')('duo:gh-resolve:github');
var request = require('co-request');
var path = require('path');
var request = require('request');

module.exports = function (opts) {
var token = opts.token || opts.password;

return {
tags: tags.bind(null, token),
branch: branch.bind(null, token)
};
/**
* Retrieve the list of tags for a given repository.
*
* @param {String} token
* @param {Object} options
* @return {Object}
*/

exports.tags = function* (token, options) {
return yield get(token, [
'repos',
options.owner,
options.repo,
'tags'
]);
};

function tags(token, owner, repo, callback) {
return request({
url: url('repos', owner, repo, 'tags'),
qs: { access_token: token },
json: true,
headers: {
'Accept': 'application/vnd.github.quicksilver-preview+json',
'User-Agent': 'duo:gh-resolve'
},
followRedirects: true
}, handle(callback));
}
/**
* Retrieve the information about a specific branch for a repository.
*
* @param {String} token
* @param {Object} options
* @return {Object}
*/

exports.branch = function* (token, options) {
return yield get(token, [
'repos',
options.owner,
options.repo,
'branches',
options.branch
]);
};

/**
* Helper for performing an HTTP GET against the Github API.
*
* @param {String} token
* @param {Array:String} parts Pieces of the URL
* @return {Response}
*/

function* get(token, parts, attempts) {
var url = 'https://api.github.com/' + path.join.apply(path, parts);
Copy link
Contributor

Choose a reason for hiding this comment

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

will path.join() do weird stuff on windows (e.g. "https://api.github.com/foo\\bar\\")?

Copy link
Contributor

Choose a reason for hiding this comment

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

haha possibly related? duojs/duo#481

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh snap... you're probably right. We should use url.resolve instead anyways. Good catch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think this is directly related to that issue. That's probably somewhere in duo-pack, if I were to guess.

But it's still a good idea here to use url.resolve() w/o path.join() at all.

debug('--> GET %s', url);

function branch(token, owner, repo, branch, callback) {
return request({
url: url('repos', owner, repo, 'branches', branch),
var res = yield request({
url: url,
qs: { access_token: token },
json: true,
headers: {
'Accept': 'application/vnd.github.quicksilver-preview+json',
'User-Agent': 'duo:gh-resolve'
},
followRedirects: true
}, handle(callback));
}
headers: { 'User-Agent': 'duo:gh-resolve' },
followRedirects: true,
json: true
});

function url() {
var parts = [].slice.call(arguments);
return base + path.join.apply(path, parts);
}
debug('<-- %s %s', res.statusCode, url);
rateLimit(res.headers);

function handle(callback) {
return function (err, res, data) {
if (err) {
callback(err, res);
} else if (isError(res.statusCode)) {
callback(new Error(data.message));
if (isError(res.statusCode)) {
var msg = res.body.message;
debug('error', msg);

if (msg === 'Not Found' || attempts > 5) {
throw new Error(msg);
} else {
callback(null, res, data);
// retry!
return yield get(token, parts, attempts + 1);
}
};
}

return res;
}

/**
* Helper for determining if a status code is an error.
* (ie: 4xx or 5xx)
*
* @param {Number} code
* @return {Boolean}
*/

function isError(code) {
var type = Math.floor(code / 100);
return type === 4 || type === 5;
}

/**
* Outputs rate-limit information via debug.
*
* @param {Object} headers
* @api private
*/

function rateLimit(headers) {
var remaining = headers['x-ratelimit-remaining'];
var limit = headers['x-ratelimit-limit'];
var reset = new Date(headers['x-ratelimit-reset'] * 1000);
debug('rate limit status: %d / %d (resets: %s)', remaining, limit, reset);
}
Loading