Skip to content

Commit f8f1121

Browse files
joyeecheungjoyeecheung
committed
implement get reviewers
1 parent 2f3843c commit f8f1121

File tree

8 files changed

+263
-61
lines changed

8 files changed

+263
-61
lines changed

‎index.js

+144-19
Original file line numberDiff line numberDiff line change
@@ -3,43 +3,168 @@
33
const fs = require('fs');
44

55
const PR_QUERY = fs.readFileSync('./queries/PR.gql', 'utf8');
6-
const request = require('./lib/request');
6+
const REVIEWS_QUERY = fs.readFileSync('./queries/Reviews.gql', 'utf8');
7+
const COMMENTS_QUERY = fs.readFileSync('./queries/PRComments.gql', 'utf8');
8+
const COMMITS_QUERY = fs.readFileSync('./queries/PRCommits.gql', 'utf8');
9+
const { request, requestAll } = require('./lib/request');
10+
const { getCollaborators } = require('./lib/collaborators');
711
const logger = require('./lib/logger');
8-
const PR_ID = parseInt(process.argv[2]) || 12756;
12+
const { ascending } = require('./lib/comp');
13+
const PR_ID = parseInt(process.argv[2]) || 14782;
914
const OWNER = 'nodejs';
1015
const REPO = 'node';
1116

12-
function getReviewers(pr) {
13-
return []; // TODO
17+
const FIXES_RE = /Fixes: (\S+)/mg;
18+
const FIX_RE = /Fixes: (\S+)/;
19+
const REFS_RE = /Refs?: (\S+)/mg;
20+
const REF_RE = /Refs?: (\S+)/;
21+
const LGTM_RE = /(\W|^)lgtm(\W|$)/i;
22+
23+
// const REFERENCE_RE = /referenced this pull request in/
24+
25+
const {
26+
PENDING, COMMENTED, APPROVED, CHANGES_REQUESTED, DISMISSED
27+
} = require('./lib/review_state');
28+
29+
function mapByGithubReviews(reviews, collaborators) {
30+
const map = new Map();
31+
const list = reviews
32+
.filter((r) => r.state !== PENDING && r.state !== COMMENTED)
33+
.filter((r) => {
34+
return (r.author && r.author.login && // could be a ghost
35+
collaborators.get(r.author.login.toLowerCase()));
36+
}).sort((a, b) => {
37+
return ascending(a.publishedAt, b.publishedAt);
38+
});
39+
40+
for (const r of list) {
41+
const login = r.author.login.toLowerCase();
42+
const entry = map.get(login);
43+
if (!entry) { // initialize
44+
map.set(login, {
45+
state: r.state,
46+
date: r.publishedAt,
47+
ref: r.url
48+
});
49+
}
50+
switch (r.state) {
51+
case APPROVED:
52+
case CHANGES_REQUESTED:
53+
// Overwrite previous, whether initalized or not
54+
map.set(login, {
55+
state: r.state,
56+
date: r.publishedAt,
57+
ref: r.url
58+
});
59+
break;
60+
case DISMISSED:
61+
// TODO: check the state of the dismissed review?
62+
map.delete(login);
63+
break;
64+
}
65+
}
66+
return map;
1467
}
1568

16-
function getFixes(pr) {
69+
// TODO: count -1 ...? But they should make it explicit
70+
/**
71+
* @param {Map} oldMap
72+
* @param {{}[]} comments
73+
* @param {Map} collaborators
74+
* @returns {Map}
75+
*/
76+
function updateMapByRawReviews(oldMap, comments, collaborators) {
77+
const withLgtm = comments.filter((c) => LGTM_RE.test(c.bodyText))
78+
.filter((c) => {
79+
return (c.author && c.author.login && // could be a ghost
80+
collaborators.get(c.author.login.toLowerCase()));
81+
}).sort((a, b) => {
82+
return ascending(a.publishedAt, b.publishedAt);
83+
});
84+
85+
for (const c of withLgtm) {
86+
const login = c.author.login.toLowerCase();
87+
const entry = oldMap.get(login);
88+
if (!entry || entry.publishedAt < c.publishedAt) {
89+
logger.info(`${login} approved via LGTM in comments`);
90+
oldMap.set(login, {
91+
state: APPROVED,
92+
date: c.publishedAt,
93+
ref: c.bodyText // no url, have to use bodyText
94+
});
95+
}
96+
}
97+
return oldMap;
98+
}
99+
/**
100+
* @param {{}[]} reviewes
101+
* @param {{}[]} comments
102+
* @param {Map} collaborators
103+
* @returns {Map}
104+
*/
105+
async function getReviewers(reviews, comments, collaborators) {
106+
const ghReviews = mapByGithubReviews(reviews, collaborators);
107+
const reviewers = updateMapByRawReviews(ghReviews, comments, collaborators);
108+
const result = [];
109+
for (const [ login, review ] of reviewers) {
110+
if (review.state !== APPROVED) {
111+
logger.warn(`${login}: ${review.state} ${review.ref}`);
112+
} else {
113+
const data = collaborators.get(login);
114+
result.push({
115+
name: data.name,
116+
email: data.email
117+
});
118+
}
119+
}
120+
return result;
121+
}
122+
123+
async function getFixes(pr) {
17124
return []; // TODO
18125
}
19126

20-
function getRefs(pr) {
127+
async function getRefs(pr) {
21128
return []; // TODO
22129
}
23130

24131
async function main() {
25-
logger.info(`Requesting ${OWNER}/${REPO}/pull/${PR_ID}`);
26-
const data = await request(PR_QUERY, {
27-
prid: PR_ID,
28-
owner: OWNER,
29-
repo: REPO
30-
});
31-
32-
const pr = data.repository.pullRequest;
132+
const prid = PR_ID;
133+
const owner = OWNER;
134+
const repo = REPO;
135+
136+
logger.info(`Requesting ${owner}/${repo}/pull/${prid}`);
137+
const prData = await request(PR_QUERY, { prid, owner, repo });
138+
const pr = prData.repository.pullRequest;
139+
const prUrl = pr.url;
140+
141+
const vars = { prid, owner, repo };
142+
logger.info(`Requesting ${owner}/${repo}/pull/${prid}/reviews`);
143+
const reviews = await requestAll(REVIEWS_QUERY, vars, [
144+
'repository', 'pullRequest', 'reviews'
145+
]);
146+
logger.info(`Requesting ${owner}/${repo}/pull/${prid}/comments`);
147+
const comments = await requestAll(COMMENTS_QUERY, vars, [
148+
'repository', 'pullRequest', 'comments'
149+
]);
150+
const collaborators = await getCollaborators(owner, repo);
151+
// logger.info(`Requesting ${owner}/${repo}/pull/${prid}/commits`);
152+
// const commits = await requestAll(COMMITS_QUERY, vars, [
153+
// 'repository', 'pullRequest', 'commits'
154+
// ]);
155+
156+
const reviewedBy = await getReviewers(reviews, comments, collaborators);
157+
const fixes = await getFixes(reviews, comments);
158+
const refs = await getRefs(reviews, comments);
159+
33160
const output = {
34-
prUrl: pr.url,
35-
reviewedBy: getReviewers(pr),
36-
fixes: getFixes(pr),
37-
refs: getRefs(pr)
161+
prUrl, reviewedBy, fixes, refs
38162
};
39163

40164
let meta = [
41165
'-------------------------------- >8 --------------------------------',
42-
`PR-URL: ${output.prUrl}`];
166+
`PR-URL: ${output.prUrl}`
167+
];
43168
meta = meta.concat(output.reviewedBy.map((reviewer) => {
44169
return `Reviewed-By: ${reviewer.name} <${reviewer.email}>`;
45170
}));

‎lib/comp.js

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
'use strict';
2+
3+
exports.ascending = function(a, b) {
4+
return a < b ? -1 : 1;
5+
};
6+
7+
exports.descending = function(a, b) {
8+
return a > b ? -1 : 1;
9+
};

‎lib/request.js

+34-1
Original file line numberDiff line numberDiff line change
@@ -32,4 +32,37 @@ async function request(query, variables) {
3232
return result.data;
3333
}
3434

35-
module.exports = request;
35+
async function requestAll(query, variables, path) {
36+
let after = null;
37+
let all = [];
38+
let totalCount;
39+
// first page
40+
do {
41+
const varWithPage = Object.assign({
42+
after
43+
}, variables);
44+
const data = await request(query, varWithPage);
45+
let current = data;
46+
for (const step of path) {
47+
current = current[step];
48+
}
49+
// current should have:
50+
// totalCount
51+
// pageInfo { hasNextPage, endCursor }
52+
// nodes
53+
all = all.concat(current.nodes);
54+
totalCount = current.totalCount;
55+
if (current.pageInfo.hasNextPage) {
56+
after = current.pageInfo.endCursor;
57+
} else {
58+
after = null;
59+
}
60+
} while (after !== null);
61+
62+
return all;
63+
}
64+
65+
module.exports = {
66+
request,
67+
requestAll
68+
};

‎lib/review_state.js

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
'use strict';
2+
3+
module.exports = {
4+
PENDING: 'PENDING',
5+
COMMENTED: 'COMMENTED',
6+
APPROVED: 'APPROVED',
7+
CHANGES_REQUESTED: 'CHANGES_REQUESTED',
8+
DISMISSED: 'DISMISSED'
9+
};

‎queries/PR.gql

+1-41
Original file line numberDiff line numberDiff line change
@@ -2,47 +2,7 @@ query PR($prid: Int!, $owner: String!, $repo: String!) {
22
repository(owner: $owner, name: $repo) {
33
pullRequest(number: $prid) {
44
url,
5-
bodyText,
6-
commits(first: 100) {
7-
totalCount
8-
nodes {
9-
commit {
10-
committedDate
11-
author {
12-
email
13-
name
14-
}
15-
committer {
16-
email
17-
name
18-
}
19-
oid
20-
message
21-
authoredByCommitter
22-
}
23-
}
24-
}
25-
comments(first: 100) {
26-
totalCount
27-
nodes {
28-
bodyText
29-
author {
30-
login
31-
}
32-
}
33-
}
34-
reviews(first: 100) {
35-
totalCount
36-
nodes {
37-
state
38-
author {
39-
login
40-
resourcePath
41-
}
42-
authorAssociation
43-
publishedAt
44-
}
45-
}
5+
bodyText
466
}
477
}
488
}

‎queries/PRComments.gql

+20
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
query Comments($prid: Int!, $owner: String!, $repo: String!, $after: String) {
2+
repository(owner: $owner, name: $repo) {
3+
pullRequest(number: $prid) {
4+
comments(first: 100, after: $after) {
5+
totalCount
6+
pageInfo {
7+
hasNextPage
8+
endCursor
9+
}
10+
nodes {
11+
publishedAt
12+
bodyText
13+
author {
14+
login
15+
}
16+
}
17+
}
18+
}
19+
}
20+
}

‎queries/PRCommits.gql

+25
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
query Commits($prid: Int!, $owner: String!, $repo: String!, $after: String) {
2+
repository(owner: $owner, name: $repo) {
3+
pullRequest(number: $prid) {
4+
commits(first: 100, after: $after) {
5+
totalCount
6+
nodes {
7+
commit {
8+
committedDate
9+
author {
10+
email
11+
name
12+
}
13+
committer {
14+
email
15+
name
16+
}
17+
oid
18+
message
19+
authoredByCommitter
20+
}
21+
}
22+
}
23+
}
24+
}
25+
}

‎queries/Reviews.gql

+21
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
query Reviews($prid: Int!, $owner: String!, $repo: String!, $after: String) {
2+
repository(owner: $owner, name: $repo) {
3+
pullRequest(number: $prid) {
4+
reviews(first: 100, after: $after) {
5+
totalCount
6+
pageInfo {
7+
hasNextPage
8+
endCursor
9+
}
10+
nodes {
11+
state
12+
author {
13+
login
14+
}
15+
url
16+
publishedAt
17+
}
18+
}
19+
}
20+
}
21+
}

0 commit comments

Comments
 (0)