Skip to content

Commit

Permalink
refactor to use pr_data
Browse files Browse the repository at this point in the history
  • Loading branch information
joyeecheung committed Nov 2, 2017
1 parent 8084817 commit 78d4d52
Show file tree
Hide file tree
Showing 8 changed files with 214 additions and 124 deletions.
57 changes: 7 additions & 50 deletions bin/metadata.js
Original file line number Diff line number Diff line change
@@ -1,24 +1,10 @@
#!/usr/bin/env node
'use strict';

const fs = require('fs');
const path = require('path');
const { EOL } = require('os');

function loadQuery(file) {
const filePath = path.resolve(__dirname, '..', 'queries', `${file}.gql`);
return fs.readFileSync(filePath, 'utf8');
}
const PR_QUERY = loadQuery('PR');
const REVIEWS_QUERY = loadQuery('Reviews');
const COMMENTS_QUERY = loadQuery('PRComments');
const COMMITS_QUERY = loadQuery('PRCommits');
const USER_QUERY = loadQuery('User');

const { request, requestAll } = require('../lib/request');
const { getCollaborators } = require('../lib/collaborators');
const req = require('../lib/request');
const loggerFactory = require('../lib/logger');
const { ReviewAnalyzer } = require('../lib/reviews');
const PRData = require('../lib/pr_data');
const PRChecker = require('../lib/pr_checker');
const MetadataGenerator = require('../lib/metadata_gen');

Expand All @@ -29,38 +15,11 @@ const OWNER = process.argv[3] || 'nodejs';
const REPO = process.argv[4] || 'node';

async function main(prid, owner, repo, logger) {
logger.trace(`Getting collaborator contacts from README of ${owner}/${repo}`);
const collaborators = await getCollaborators(logger, owner, repo);

logger.trace(`Getting PR from ${owner}/${repo}/pull/${prid}`);
const prData = await request(PR_QUERY, { prid, owner, repo });
const pr = prData.repository.pullRequest;

// Get the mail
logger.trace(`Getting User information for ${pr.author.login}`);
const userData = await request(USER_QUERY, { login: pr.author.login });
const user = userData.user;
Object.assign(pr.author, user);

const vars = { prid, owner, repo };
logger.trace(`Getting reviews from ${owner}/${repo}/pull/${prid}`);
const reviews = await requestAll(REVIEWS_QUERY, vars, [
'repository', 'pullRequest', 'reviews'
]);
logger.trace(`Getting comments from ${owner}/${repo}/pull/${prid}`);
const comments = await requestAll(COMMENTS_QUERY, vars, [
'repository', 'pullRequest', 'comments'
]);

logger.trace(`Getting commits from ${owner}/${repo}/pull/${prid}`);
const commits = await requestAll(COMMITS_QUERY, vars, [
'repository', 'pullRequest', 'commits'
]);

const analyzer = new ReviewAnalyzer(reviews, comments, collaborators);
const reviewers = analyzer.getReviewers();
const metadata = new MetadataGenerator(repo, pr, reviewers).getMetadata();
const data = new PRData(prid, owner, repo, logger, req);
await data.getAll();
data.analyzeReviewers();

const metadata = new MetadataGenerator(data).getMetadata();
const [SCISSOR_LEFT, SCISSOR_RIGHT] = MetadataGenerator.SCISSORS;
logger.info({
raw: [SCISSOR_LEFT, metadata, SCISSOR_RIGHT].join(EOL)
Expand All @@ -69,12 +28,10 @@ async function main(prid, owner, repo, logger) {
if (!process.stdout.isTTY) {
process.stdout.write(`${metadata}${EOL}`);
}

/**
* TODO: put all these data into one object with a class
*/
const checker = new PRChecker(logger, pr, reviewers, comments, reviews,
commits, collaborators);
const checker = new PRChecker(logger, data);
checker.checkAll();
}

Expand Down
17 changes: 9 additions & 8 deletions lib/metadata_gen.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,10 @@ const { EOL } = require('os');
*/
class MetadataGenerator {
/**
* @param {string} repo
* @param {{url: string, bodyHTML: string}} pr
* @param {{approved: Reviewer[], rejected: Reviewer[]}} reviewers
* @param {PRData} data
*/
constructor(repo, pr, reviewers) {
constructor(data) {
const { repo, pr, reviewers } = data;
this.repo = repo;
this.pr = pr;
this.reviewers = reviewers;
Expand All @@ -21,11 +20,13 @@ class MetadataGenerator {
* @returns {string}
*/
getMetadata() {
const { reviewers, repo, pr } = this;
const {
reviewers: { approved: reviewedBy },
pr: { url: prUrl, bodyHTML: op },
repo
} = this;

const prUrl = pr.url;
const reviewedBy = reviewers.approved;
const parser = new LinkParser(repo, pr.bodyHTML);
const parser = new LinkParser(repo, op);
const fixes = parser.getFixes();
const refs = parser.getRefs();

Expand Down
9 changes: 8 additions & 1 deletion lib/pr_checker.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,15 @@ const CI_TYPES = CIParser.TYPES;
const { FULL } = CIParser.constants;

class PRChecker {
constructor(logger, pr, reviewers, comments, reviews, commits, collaborators) {
/**
* @param {{}} logger
* @param {PRData} data
*/
constructor(logger, data) {
this.logger = logger;
const {
pr, reviewers, comments, reviews, commits, collaborators
} = data;
this.reviewers = reviewers;
this.pr = pr;
this.comments = comments;
Expand Down
106 changes: 106 additions & 0 deletions lib/pr_data.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
'use strict';

const fs = require('fs');
const path = require('path');

function loadQuery(file) {
const filePath = path.resolve(__dirname, '..', 'queries', `${file}.gql`);
return fs.readFileSync(filePath, 'utf8');
}

const PR_QUERY = loadQuery('PR');
const REVIEWS_QUERY = loadQuery('Reviews');
const COMMENTS_QUERY = loadQuery('PRComments');
const COMMITS_QUERY = loadQuery('PRCommits');
const USER_QUERY = loadQuery('User');

// TODO(joyeecheung): make it mockable with req.rp ?
const { getCollaborators } = require('../lib/collaborators');
const { ReviewAnalyzer } = require('../lib/reviews');

class PRData {
/**
* @param {number} prid
* @param {string} owner
* @param {string} repo
* @param {*} logger
* @param {{request: Function, requestAll: Function}} req
*/
constructor(prid, owner, repo, logger, req) {
this.prid = prid;
this.owner = owner;
this.repo = repo;
this.logger = logger;
this.request = req.request;
this.requestAll = req.requestAll;

// Data
this.collaborators = new Map();
this.pr = {};
this.reviews = [];
this.comments = [];
this.commits = [];
this.reviewers = [];
}

async getAll() {
await Promise.all([
this.getCollaborators(),
this.getPR(),
this.getReviews(),
this.getComments(),
this.getCommits()
]);
}

analyzeReviewers() {
this.reviewers = new ReviewAnalyzer(this).getReviewers();
}

async getCollaborators() {
const { owner, repo, logger } = this;
logger.trace(`Getting collaborator contacts from README of ${owner}/${repo}`);
this.collaborators = await getCollaborators(logger, owner, repo);
}

async getPR() {
const { prid, owner, repo, logger, request } = this;
logger.trace(`Getting PR from ${owner}/${repo}/pull/${prid}`);
const prData = await request(PR_QUERY, { prid, owner, repo });
const pr = this.pr = prData.repository.pullRequest;
// Get the mail
logger.trace(`Getting User information for ${pr.author.login}`);
const userData = await request(USER_QUERY, { login: pr.author.login });
const user = userData.user;
Object.assign(this.pr.author, user);
}

async getReviews() {
const { prid, owner, repo, logger, requestAll } = this;
const vars = { prid, owner, repo };
logger.trace(`Getting reviews from ${owner}/${repo}/pull/${prid}`);
this.reviews = await requestAll(REVIEWS_QUERY, vars, [
'repository', 'pullRequest', 'reviews'
]);
}

async getComments() {
const { prid, owner, repo, logger, requestAll } = this;
const vars = { prid, owner, repo };
logger.trace(`Getting comments from ${owner}/${repo}/pull/${prid}`);
this.comments = await requestAll(COMMENTS_QUERY, vars, [
'repository', 'pullRequest', 'comments'
]);
}

async getCommits() {
const { prid, owner, repo, logger, requestAll } = this;
const vars = { prid, owner, repo };
logger.trace(`Getting commits from ${owner}/${repo}/pull/${prid}`);
this.commits = await requestAll(COMMITS_QUERY, vars, [
'repository', 'pullRequest', 'commits'
]);
}
};

module.exports = PRData;
8 changes: 4 additions & 4 deletions lib/reviews.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,14 @@ class Review {
* @property {string} bodyText
* @property {{login: string}} author
* @property {string} publishedAt
*
*/
class ReviewAnalyzer {
/**
* @param {GHReview[]} reviews
* @param {GHComment[]} comments
* @param {Map<string, Collaborator>} collaborators
* @param {PRData} data
*/
constructor(reviews, comments, collaborators) {
constructor(data) {
const { reviews, comments, collaborators } = data;
this.reviews = reviews;
this.comments = comments;
this.collaborators = collaborators;
Expand Down
9 changes: 6 additions & 3 deletions test/unit/metadata_gen.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,12 @@ const { Collaborator } = require('../../lib/collaborators');

const approved = readJSON('reviewers_approved.json');
patchPrototype(approved, 'reviewer', Collaborator.prototype);
const reviewers = { approved, rejected: [] };

const pr = readJSON('pr_with_fixes_and_refs.json');
const data = {
repo: 'node',
pr,
reviewers: { approved, rejected: [] }
};

const expected = `PR-URL: https://github.com/nodejs/node/pull/16438
Fixes: https://github.com/node/issues/16437
Expand All @@ -20,7 +23,7 @@ Reviewed-By: Bar User <[email protected]>`;

describe('MetadataGenerator', () => {
it('should generate metadata properly', () => {
const results = new MetadataGenerator('node', pr, reviewers).getMetadata();
const results = new MetadataGenerator(data).getMetadata();
assert.strictEqual(expected, results);
});
});
Loading

0 comments on commit 78d4d52

Please sign in to comment.