Skip to content

Commit 5186451

Browse files
committed
MailArchiveGitHelper: do not allow the config to be passed implicitly
Implicit configuration is a recipe for confusion when trying to allow for overriding said configuration. Let's require the config to be passed down explicitly. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
1 parent e8bb14c commit 5186451

File tree

3 files changed

+60
-14
lines changed

3 files changed

+60
-14
lines changed

lib/ci-helper.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1110,6 +1110,7 @@ export class CIHelper {
11101110
};
11111111
await this.maybeUpdateGGGNotes();
11121112
const mailArchiveGit = await MailArchiveGitHelper.get(
1113+
this.config,
11131114
this.notes,
11141115
mailArchiveGitDir,
11151116
this.github,

lib/mail-archive-helper.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { IGitGitGadgetOptions } from "./gitgitgadget.js";
55
import { GitHubGlue } from "./github-glue.js";
66
import { IMailMetadata } from "./mail-metadata.js";
77
import { IPatchSeriesMetadata } from "./patch-series-metadata.js";
8-
import { IConfig, getConfig } from "./project-config.js";
8+
import { IConfig } from "./project-config.js";
99
import { getPullRequestKey } from "./pullRequestKey.js";
1010
import { IParsedMBox, parseMBox, parseMBoxMessageIDAndReferences } from "./send-mail.js";
1111
import { SousChef } from "./sous-chef.js";
@@ -19,13 +19,14 @@ export interface IGitMailingListMirrorState {
1919

2020
export class MailArchiveGitHelper {
2121
public static async get(
22+
config: IConfig,
2223
gggNotes: GitNotes,
2324
mailArchiveGitDir: string,
2425
githubGlue: GitHubGlue,
2526
branch: string,
2627
): Promise<MailArchiveGitHelper> {
2728
const state: IGitMailingListMirrorState = (await gggNotes.get<IGitMailingListMirrorState>(stateKey)) || {};
28-
return new MailArchiveGitHelper(gggNotes, mailArchiveGitDir, githubGlue, state, branch);
29+
return new MailArchiveGitHelper(config, gggNotes, mailArchiveGitDir, githubGlue, state, branch);
2930
}
3031

3132
/**
@@ -56,19 +57,21 @@ export class MailArchiveGitHelper {
5657
}
5758

5859
protected readonly branch: string;
59-
protected readonly config: IConfig = getConfig();
60+
protected readonly config: IConfig;
6061
protected readonly state: IGitMailingListMirrorState;
6162
protected readonly gggNotes: GitNotes;
6263
protected readonly mailArchiveGitDir: string;
6364
protected readonly githubGlue: GitHubGlue;
6465

6566
protected constructor(
67+
config: IConfig,
6668
gggNotes: GitNotes,
6769
mailArchiveGitDir: string,
6870
githubGlue: GitHubGlue,
6971
state: IGitMailingListMirrorState,
7072
branch: string,
7173
) {
74+
this.config = config;
7275
this.branch = branch;
7376
this.gggNotes = gggNotes;
7477
this.mailArchiveGitDir = mailArchiveGitDir;

tests/mail-archive-helper.test.ts

Lines changed: 53 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,24 +3,25 @@ import { OctokitResponse } from "@octokit/types";
33
import { expect, jest, test } from "@jest/globals";
44
import { fileURLToPath } from "url";
55
import { GitNotes } from "../lib/git-notes.js";
6-
import { getConfig } from "../lib/gitgitgadget-config.js";
76
import { GitHubGlue } from "../lib/github-glue.js";
87
import { MailArchiveGitHelper, IGitMailingListMirrorState } from "../lib/mail-archive-helper.js";
98
import { IMailMetadata } from "../lib/mail-metadata.js";
10-
import { setConfig } from "../lib/project-config.js";
119
import { testCreateRepo } from "./test-lib.js";
10+
import defaultConfig from "../lib/gitgitgadget-config.js";
11+
import { IConfig } from "../lib/project-config.js";
1212

1313
/* eslint max-classes-per-file: ["error", 2] */
1414

1515
class MailArchiveGitHelperProxy extends MailArchiveGitHelper {
1616
public constructor(
17+
config: IConfig,
1718
gggNotes: GitNotes,
1819
mailArchiveGitDir: string,
1920
githubGlue: GitHubGlue,
2021
state: IGitMailingListMirrorState,
2122
branch: string,
2223
) {
23-
super(gggNotes, mailArchiveGitDir, githubGlue, state, branch);
24+
super(config, gggNotes, mailArchiveGitDir, githubGlue, state, branch);
2425
}
2526
}
2627
class GitHubProxy extends GitHubGlue {
@@ -96,10 +97,9 @@ interface ICommit {
9697
parents: [{ sha: string; url: string; html_url?: string }];
9798
}
9899

99-
const config = getConfig();
100+
const config = { ...defaultConfig }; // make a copy
100101
config.repo.owner = "test";
101102
config.repo.name = "test";
102-
setConfig(config);
103103

104104
const fromEmail = "I Replied <ireplied@gmail.com>";
105105

@@ -205,7 +205,14 @@ This Pull Request contains some ipsum lorem.
205205
const notes = new GitNotes(repo.workDir);
206206
await notes.set(mailMeta.messageID, mailMeta, true);
207207

208-
const mail = new MailArchiveGitHelperProxy(notes, repo.workDir, github, { latestRevision: "HEAD~" }, "master");
208+
const mail = new MailArchiveGitHelperProxy(
209+
config,
210+
notes,
211+
repo.workDir,
212+
github,
213+
{ latestRevision: "HEAD~" },
214+
"master",
215+
);
209216

210217
const logSpy = jest.spyOn(console, "log").mockImplementation(() => {});
211218
await mail.processMails();
@@ -240,7 +247,14 @@ This Pull Request contains some ipsum lorem.
240247
const notes = new GitNotes(repo.workDir);
241248
await notes.set(mailMeta.messageID, mailMeta, true);
242249

243-
const mail = new MailArchiveGitHelperProxy(notes, repo.workDir, github, { latestRevision: "HEAD~" }, "master");
250+
const mail = new MailArchiveGitHelperProxy(
251+
config,
252+
notes,
253+
repo.workDir,
254+
github,
255+
{ latestRevision: "HEAD~" },
256+
"master",
257+
);
244258

245259
const logSpy = jest.spyOn(console, "log").mockImplementation(() => {});
246260
await mail.processMails();
@@ -279,7 +293,14 @@ This Pull Request contains some ipsum lorem.
279293
const notes = new GitNotes(repo.workDir);
280294
await notes.set(mailMeta.messageID, mailMeta, true);
281295

282-
const mail = new MailArchiveGitHelperProxy(notes, repo.workDir, github, { latestRevision: "HEAD~" }, "master");
296+
const mail = new MailArchiveGitHelperProxy(
297+
config,
298+
notes,
299+
repo.workDir,
300+
github,
301+
{ latestRevision: "HEAD~" },
302+
"master",
303+
);
283304

284305
const commitsResponse = getCommitsResponse;
285306
const fail = false;
@@ -365,7 +386,14 @@ This Pull Request contains some ipsum lorem.
365386
const notes = new GitNotes(repo.workDir);
366387
await notes.set(mailMeta.messageID, mailMeta, true);
367388

368-
const mail = new MailArchiveGitHelperProxy(notes, repo.workDir, github, { latestRevision: "HEAD~~" }, "master");
389+
const mail = new MailArchiveGitHelperProxy(
390+
config,
391+
notes,
392+
repo.workDir,
393+
github,
394+
{ latestRevision: "HEAD~~" },
395+
"master",
396+
);
369397

370398
const commitsResponse = getCommitsResponse;
371399
const fail = false;
@@ -453,7 +481,14 @@ This Pull Request contains some ipsum lorem.
453481
const notes = new GitNotes(repo.workDir);
454482
await notes.set(mailMeta.messageID, mailMeta, true);
455483

456-
const mail = new MailArchiveGitHelperProxy(notes, repo.workDir, github, { latestRevision: "HEAD~" }, "master");
484+
const mail = new MailArchiveGitHelperProxy(
485+
config,
486+
notes,
487+
repo.workDir,
488+
github,
489+
{ latestRevision: "HEAD~" },
490+
"master",
491+
);
457492

458493
const commitsResponse = getCommitsResponse;
459494
const fail = true;
@@ -530,7 +565,14 @@ This Pull Request contains some ipsum lorem.
530565
const notes = new GitNotes(repo.workDir);
531566
await notes.set(mailMeta.messageID, mailMeta, true);
532567

533-
const mail = new MailArchiveGitHelperProxy(notes, repo.workDir, github, { latestRevision: "HEAD~" }, "master");
568+
const mail = new MailArchiveGitHelperProxy(
569+
config,
570+
notes,
571+
repo.workDir,
572+
github,
573+
{ latestRevision: "HEAD~" },
574+
"master",
575+
);
534576

535577
const commitsResponse = getCommitsResponse;
536578
commitsResponse.data[0].sha = mailMeta.originalCommit;

0 commit comments

Comments
 (0)