Skip to content

Commit e4eaea2

Browse files
dschodscho
authored
Merge pull request #1198 from webstech/comment
Mail archive - track first patch line when sending patch email
2 parents 3d092fe + 2d0d92a commit e4eaea2

File tree

4 files changed

+48
-68
lines changed

4 files changed

+48
-68
lines changed

‎lib/github-glue.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -190,12 +190,13 @@ export class GitHubGlue {
190190
* @param {pullRequestKeyInfo} pullRequest - the Pull Request to comment on
191191
* @param {string} commit the hash of the commit to comment on
192192
* @param {string} comment the comment
193+
* @param {number} line the comment is referencing
193194
* @returns the comment ID and the URL to the comment
194195
*/
195196
public async addPRCommitComment(pullRequest: pullRequestKeyInfo,
196197
commit: string,
197198
gitWorkDir: string | undefined,
198-
comment: string):
199+
comment: string, line?: number | undefined):
199200
Promise<{id: number; url: string}> {
200201
const prKey = getPullRequestKey(pullRequest);
201202

@@ -210,7 +211,7 @@ export class GitHubGlue {
210211
body: comment,
211212
commit_id: commit,
212213
path,
213-
line: 1,
214+
line: line || 1,
214215
...prKey
215216
});
216217
return {

‎lib/mail-archive-helper.ts‎

Lines changed: 38 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -6,26 +6,22 @@ import { GitHubGlue } from "./github-glue";
66
import { IMailMetadata } from "./mail-metadata";
77
import { IPatchSeriesMetadata } from "./patch-series-metadata";
88
import { IConfig, getConfig } from "./project-config";
9-
import { IParsedMBox, parseMBox,
10-
parseMBoxMessageIDAndReferences } from "./send-mail";
9+
import { getPullRequestKey } from "./pullRequestKey";
10+
import { IParsedMBox, parseMBox, parseMBoxMessageIDAndReferences } from "./send-mail";
1111
import { SousChef } from "./sous-chef";
1212

1313
export const stateKey = "git@vger.kernel.org <-> GitGitGadget";
14-
const replyToThisURL =
15-
"https://github.com/gitgitgadget/gitgitgadget/wiki/ReplyToThis";
14+
const replyToThisURL = "https://github.com/gitgitgadget/gitgitgadget/wiki/ReplyToThis";
1615

1716
export interface IGitMailingListMirrorState {
1817
latestRevision?: string;
1918
}
2019

2120
export class MailArchiveGitHelper {
22-
public static async get(gggNotes: GitNotes, mailArchiveGitDir: string,
23-
githubGlue: GitHubGlue, branch: string):
21+
public static async get(gggNotes: GitNotes, mailArchiveGitDir: string, githubGlue: GitHubGlue, branch: string):
2422
Promise<MailArchiveGitHelper> {
25-
const state: IGitMailingListMirrorState =
26-
await gggNotes.get<IGitMailingListMirrorState>(stateKey) || {};
27-
return new MailArchiveGitHelper(gggNotes, mailArchiveGitDir, githubGlue,
28-
state, branch);
23+
const state: IGitMailingListMirrorState = await gggNotes.get<IGitMailingListMirrorState>(stateKey) || {};
24+
return new MailArchiveGitHelper(gggNotes, mailArchiveGitDir, githubGlue, state, branch);
2925
}
3026

3127
/**
@@ -62,19 +58,16 @@ export class MailArchiveGitHelper {
6258
protected readonly mailArchiveGitDir: string;
6359
protected readonly githubGlue: GitHubGlue;
6460

65-
protected constructor(gggNotes: GitNotes, mailArchiveGitDir: string,
66-
githubGlue: GitHubGlue,
67-
state: IGitMailingListMirrorState,
68-
branch: string) {
61+
protected constructor(gggNotes: GitNotes, mailArchiveGitDir: string, githubGlue: GitHubGlue,
62+
state: IGitMailingListMirrorState, branch: string) {
6963
this.branch = branch;
7064
this.gggNotes = gggNotes;
7165
this.mailArchiveGitDir = mailArchiveGitDir;
7266
this.githubGlue = githubGlue;
7367
this.state = state;
7468
}
7569

76-
public async processMails(prFilter?: (pullRequestURL: string) => boolean):
77-
Promise<boolean> {
70+
public async processMails(prFilter?: (pullRequestURL: string) => boolean): Promise<boolean> {
7871
const keys: Set<string> = new Set<string>();
7972
(await git(["ls-tree", "-r", `${this.gggNotes.notesRef}:`],
8073
{ workDir: this.gggNotes.workDir })).split("\n")
@@ -99,11 +92,9 @@ export class MailArchiveGitHelper {
9992
if (prFilter && !prFilter(pullRequestURL)) {
10093
continue;
10194
}
102-
const prMeta = await this.gggNotes
103-
.get<IPatchSeriesMetadata>(pullRequestURL);
95+
const prMeta = await this.gggNotes.get<IPatchSeriesMetadata>(pullRequestURL);
10496
if (prMeta && prMeta.branchNameInGitsterGit) {
105-
branchNameMap.set(prMeta.branchNameInGitsterGit,
106-
pullRequestURL);
97+
branchNameMap.set(prMeta.branchNameInGitsterGit, pullRequestURL);
10798
}
10899
}
109100
const sousChef = new SousChef(mbox);
@@ -115,35 +106,21 @@ export class MailArchiveGitHelper {
115106
for (const branchName of sousChef.branches.keys()) {
116107
const pullRequestURL = branchNameMap.get(branchName);
117108
if (pullRequestURL) {
118-
const branchBaseURL
119-
= "https://github.com/gitgitgadget/git/commits/";
109+
const branchBaseURL = "https://github.com/gitgitgadget/git/commits/";
120110
const info = sousChef.branches.get(branchName);
121-
const pre = info?.text
122-
.replace(/&/g, "&amp;")
123-
.replace(/</g, "&lt;").replace(/>/g, "&gt;");
111+
const pre = info?.text.replace(/&/g, "&amp;").replace(/</g, "&lt;").replace(/>/g, "&gt;");
124112
let comment: string;
125113
if (!pre || pre.trim() === "") {
126-
comment = `The branch [\`${
127-
branchName}\`](${
128-
branchBaseURL}${
129-
branchName}) was mentioned in the "${
130-
info?.sectionName
131-
}" section of the [status updates](${
132-
whatsCookingBaseURL}${
133-
sousChef.messageID}) on the Git mailing list.`;
114+
comment = `The branch [\`${branchName}\`](${branchBaseURL}${
115+
branchName}) was mentioned in the "${info?.sectionName}" section of the [status updates](${
116+
whatsCookingBaseURL}${sousChef.messageID}) on the Git mailing list.`;
134117
} else {
135-
comment = `There was a [status update](${
136-
whatsCookingBaseURL}${
137-
sousChef.messageID}) in the "${
138-
info?.sectionName}" section about the branch [\`${
139-
branchName}\`](${
140-
branchBaseURL}${
141-
branchName}) on the Git mailing list:\n\n<pre>\n${
142-
pre}\n</pre>`;
118+
comment = `There was a [status update](${whatsCookingBaseURL}${sousChef.messageID}) in the "${
119+
info?.sectionName}" section about the branch [\`${branchName}\`](${branchBaseURL}${
120+
branchName}) on the Git mailing list:\n\n<pre>\n${pre}\n</pre>`;
143121
}
144122
console.log(`\n${pullRequestURL}: ${comment}`);
145-
await this.githubGlue
146-
.addPRComment(pullRequestURL, comment);
123+
await this.githubGlue.addPRComment(pullRequestURL, comment);
147124
}
148125
}
149126
};
@@ -169,21 +146,20 @@ export class MailArchiveGitHelper {
169146
let pullRequestURL: string | undefined;
170147
let originalCommit: string | undefined;
171148
let issueCommentId: number | undefined;
149+
let firstPatchLine: number | undefined;
172150
for (const reference of parsed.references.filter(seen)) {
173-
const data =
174-
await this.gggNotes.get<IMailMetadata>(reference);
151+
const data = await this.gggNotes.get<IMailMetadata>(reference);
175152
if (data && data.pullRequestURL) {
176153
if (prFilter && !prFilter(data.pullRequestURL)) {
177154
continue;
178155
}
179156
/* Cover letters were recorded with their tip commits */
180-
const commit = reference.match(/^pull/) ?
181-
undefined : data.originalCommit;
182-
if (!pullRequestURL ||
183-
(!originalCommit && commit) ||
157+
const commit = reference.match(/^pull/) ? undefined : data.originalCommit;
158+
if (!pullRequestURL || (!originalCommit && commit) ||
184159
(!issueCommentId && data.issueCommentId)) {
185160
pullRequestURL = data.pullRequestURL;
186161
issueCommentId = data.issueCommentId;
162+
firstPatchLine = data.firstPatchLine;
187163
originalCommit = commit;
188164
}
189165
}
@@ -198,25 +174,24 @@ export class MailArchiveGitHelper {
198174
}, comment ID: ${issueCommentId}`);
199175

200176
const archiveURL = `${this.config.mailrepo.url}${parsed.messageID}`;
201-
const header = `[On the Git mailing list](${archiveURL}), ` +
202-
(parsedMbox.from ?
203-
parsedMbox.from.replace(/ *<.*>/, "") : "Somebody") +
204-
` wrote ([reply to this](${replyToThisURL})):\n\n`;
177+
const header = `[On the Git mailing list](${archiveURL}), ${
178+
parsedMbox.from ? parsedMbox.from.replace(/ *<.*>/, "") : "Somebody"
179+
} wrote ([reply to this](${replyToThisURL})):\n\n`;
205180
const comment = MailArchiveGitHelper.mbox2markdown(parsedMbox);
206181
const fullComment = header + comment;
182+
const prKey = getPullRequestKey(pullRequestURL);
207183

208184
if (issueCommentId) {
209-
await this.githubGlue.addPRCommentReply(pullRequestURL,
210-
issueCommentId,
211-
fullComment);
185+
await this.githubGlue.addPRCommentReply(pullRequestURL, issueCommentId, fullComment);
212186
} else if (originalCommit) {
213187
try {
214-
const result = await this.githubGlue
215-
.addPRCommitComment(pullRequestURL, originalCommit,
216-
this.gggNotes.workDir, fullComment);
188+
const result = await this.githubGlue.addPRCommitComment(pullRequestURL, originalCommit,
189+
this.gggNotes.workDir, fullComment, firstPatchLine);
217190
issueCommentId = result.id;
218191
} catch (error) {
219-
const regarding = `${header.slice(0,-3)}, regarding ${originalCommit}:\n\n`;
192+
const commits = await this.githubGlue.getPRCommits(prKey.owner, prKey.pull_number);
193+
const regarding = `${header.slice(0,-3)}, regarding ${originalCommit}${
194+
commits.find(e => e.commit === originalCommit) ? "" : " (outdated)"}:\n\n`;
220195
await this.githubGlue.addPRComment(pullRequestURL, regarding + comment);
221196
originalCommit = undefined;
222197
}
@@ -225,12 +200,10 @@ export class MailArchiveGitHelper {
225200
* We will not use the ID of this comment, as it is an
226201
* issue comment, really, not a Pull Request comment.
227202
*/
228-
await this.githubGlue
229-
.addPRComment(pullRequestURL, fullComment);
203+
await this.githubGlue.addPRComment(pullRequestURL, fullComment);
230204
}
231205

232-
await this.githubGlue.addPRCc(pullRequestURL,
233-
parsedMbox.from || "");
206+
await this.githubGlue.addPRCc(pullRequestURL, parsedMbox.from || "");
234207

235208
await this.gggNotes.set(parsed.messageID, {
236209
issueCommentId,
@@ -281,8 +254,7 @@ export class MailArchiveGitHelper {
281254

282255
const range = `${this.state.latestRevision}..${head}`;
283256
console.log(`Handling commit range ${range}`);
284-
await git(["log", "-p", "-U99999", "--reverse", range],
285-
{ lineHandler, workDir: this.mailArchiveGitDir });
257+
await git(["log", "-p", "-U99999", "--reverse", range], { lineHandler, workDir: this.mailArchiveGitDir });
286258

287259
this.state.latestRevision = head;
288260
await this.gggNotes.set(stateKey, this.state, true);

‎lib/mail-metadata.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,5 @@ export interface IMailMetadata {
44
issueCommentId?: number;
55
originalCommit?: string;
66
commitInGitGit?: string;
7+
firstPatchLine?: number;
78
}

‎lib/patch-series.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -823,20 +823,26 @@ export class PatchSeries {
823823
const messageID = mail.match(/\nMessage-ID: <(.*?)>\n/i);
824824
if (messageID) {
825825
let originalCommit: string | undefined;
826+
let firstPatchLine: number | undefined;
826827
if (isCoverLetter) {
827828
isCoverLetter = false;
828829
} else {
829830
const commitMatch = mail.match(/^From ([0-9a-f]{40}) /);
830831
if (commitMatch) {
831832
originalCommit = commitMatch[1];
832833
}
834+
const revLine = mail.match(/\n@@ -(\d+),/);
835+
if (revLine) {
836+
firstPatchLine = parseInt(revLine[1], 10);
837+
}
833838
}
834839

835840
const mid = messageID[1];
836841
const mailMeta = {
837842
messageID: mid,
838843
originalCommit,
839844
pullRequestURL: this.metadata.pullRequestURL,
845+
firstPatchLine
840846
} as IMailMetadata;
841847
await this.notes.set(mid, mailMeta, true);
842848
if (globalOptions && originalCommit &&

0 commit comments

Comments
 (0)