Skip to content

Commit 4402c0d

Browse files
committed
CIHelper: make the config an explicit constructor parameter
It was a bit hard for me to follow the code, getting confused by two separate `getConfig()` functions that do different things (the one from `gitgitgadget-config` initializes the default config, the one from `project-config` gets a singleton that might still be uninitialized). Let's unconfuse things by making the config a parameter of the constructor. It can still be omitted, in which case the default config is used. Note: This commit is only concerned with the CIHelper class, because we will use it from light-weight GitHub Actions in the future. The rest of the code really needs to use the singleton up until the time when we'll get around to The Big Refactor. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
1 parent 3105d8f commit 4402c0d

File tree

6 files changed

+24
-16
lines changed

6 files changed

+24
-16
lines changed

lib/ci-helper.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,14 @@ import { ILintError, LintCommit } from "./commit-lint.js";
55
import { commitExists, git, emptyTreeName } from "./git.js";
66
import { GitNotes } from "./git-notes.js";
77
import { GitGitGadget, IGitGitGadgetOptions } from "./gitgitgadget.js";
8+
import { getConfig } from "./gitgitgadget-config.js";
89
import { GitHubGlue, IGitHubUser, IPRComment, IPRCommit, IPullRequestInfo, RequestError } from "./github-glue.js";
910
import { toPrettyJSON } from "./json-util.js";
1011
import { MailArchiveGitHelper } from "./mail-archive-helper.js";
1112
import { MailCommitMapping } from "./mail-commit-mapping.js";
1213
import { IMailMetadata } from "./mail-metadata.js";
1314
import { IPatchSeriesMetadata } from "./patch-series-metadata.js";
14-
import { IConfig, getConfig } from "./project-config.js";
15+
import { IConfig, getExternalConfig, setConfig } from "./project-config.js";
1516
import { getPullRequestKeyFromURL, pullRequestKey } from "./pullRequestKey.js";
1617

1718
const readFile = util.promisify(fs.readFile);
@@ -25,7 +26,7 @@ type CommentFunction = (comment: string) => Promise<void>;
2526
* commit.
2627
*/
2728
export class CIHelper {
28-
public readonly config: IConfig = getConfig();
29+
public readonly config: IConfig;
2930
public readonly workDir: string;
3031
public readonly notes: GitNotes;
3132
public readonly urlBase: string;
@@ -40,7 +41,12 @@ export class CIHelper {
4041
private notesPushToken: string | undefined;
4142
protected maxCommitsExceptions: string[];
4243

43-
public constructor(workDir: string, skipUpdate?: boolean, gggConfigDir = ".") {
44+
public static async getConfig(configFile?: string): Promise<IConfig> {
45+
return configFile ? await getExternalConfig(configFile) : getConfig();
46+
}
47+
48+
public constructor(workDir: string, config?: IConfig, skipUpdate?: boolean, gggConfigDir = ".") {
49+
this.config = config !== undefined ? setConfig(config) : getConfig();
4450
this.gggConfigDir = gggConfigDir;
4551
this.workDir = workDir;
4652
this.notes = new GitNotes(workDir);

lib/misc-action.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ export async function handleAction(parms: actionInterface): Promise<void> {
3535
throw new Error(`git WorkDir '${parms.repositoryDir}' not found.`);
3636
}
3737

38-
const ci = new CIHelper(parms.repositoryDir, parms.skipUpdate ? true : false, parms.configRepositoryDir);
38+
const ci = new CIHelper(parms.repositoryDir, config, parms.skipUpdate ? true : false, parms.configRepositoryDir);
3939

4040
if (parms.action === "update-open-prs") {
4141
const result = await ci.updateOpenPrs();

lib/pull-action.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ export async function handlePRUpdate(parms: PRUpdateInterface): Promise<void> {
3838
throw new Error(`git WorkDir '${parms.repositoryDir}' not found.`);
3939
}
4040

41-
const ci = new CIHelper(parms.repositoryDir, parms.skipUpdate ? true : false, parms.configRepositoryDir);
41+
const ci = new CIHelper(parms.repositoryDir, config, parms.skipUpdate ? true : false, parms.configRepositoryDir);
4242

4343
if (parms.action === "comment") {
4444
if (parms.commentId) {

script/misc-helper.ts

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,11 @@ import { CIHelper } from "../lib/ci-helper.js";
55
import { isDirectory } from "../lib/fs-util.js";
66
import { git, gitConfig } from "../lib/git.js";
77
import { IGitGitGadgetOptions, getVar } from "../lib/gitgitgadget.js";
8-
import { getConfig } from "../lib/gitgitgadget-config.js";
98
import { GitHubGlue } from "../lib/github-glue.js";
109
import { toPrettyJSON } from "../lib/json-util.js";
1110
import { IGitMailingListMirrorState, stateKey } from "../lib/mail-archive-helper.js";
1211
import { IPatchSeriesMetadata } from "../lib/patch-series-metadata.js";
13-
import { IConfig, getExternalConfig, setConfig } from "../lib/project-config.js";
12+
import { IConfig } from "../lib/project-config.js";
1413

1514
let commander = new Command();
1615
const publishRemoteKey = "publishRemote";
@@ -46,9 +45,7 @@ interface ICommanderOptions {
4645
const commandOptions = commander.opts<ICommanderOptions>();
4746

4847
(async (): Promise<void> => {
49-
const config: IConfig = commandOptions.config
50-
? setConfig(await getExternalConfig(commandOptions.config))
51-
: getConfig();
48+
const config: IConfig = await CIHelper.getConfig(commandOptions.config);
5249

5350
const getGitGitWorkDir = async (): Promise<string> => {
5451
if (!commandOptions.gitWorkDir) {
@@ -69,7 +66,12 @@ const commandOptions = commander.opts<ICommanderOptions>();
6966
return commandOptions.gitWorkDir;
7067
};
7168

72-
const ci = new CIHelper(await getGitGitWorkDir(), commandOptions.skipUpdate, commandOptions.gitgitgadgetWorkDir);
69+
const ci = new CIHelper(
70+
await getGitGitWorkDir(),
71+
config,
72+
commandOptions.skipUpdate,
73+
commandOptions.gitgitgadgetWorkDir,
74+
);
7375

7476
const configureNotesPushToken = async (): Promise<void> => {
7577
const token = await gitConfig("gitgitgadget.githubToken");

tests-config/ci-helper.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ class TestCIHelper extends CIHelper {
9494
public addPRLabelsCalls: Array<[_: string, labels: string[]]>;
9595

9696
public constructor(workDir: string, debug = false, gggDir = ".") {
97-
super(workDir, debug, gggDir);
97+
super(workDir, config, debug, gggDir);
9898
this.testing = true;
9999
this.ghGlue = this.github;
100100

@@ -240,7 +240,7 @@ test("identify merge that integrated some commit", async () => {
240240
const commitD = await repo.merge("d", commitF);
241241
await repo.git(["update-ref", `refs/remotes/upstream/${config.repo.trackingBranches[2]}`, commitD]);
242242

243-
const ci = new CIHelper(repo.workDir, true);
243+
const ci = new CIHelper(repo.workDir, config, true);
244244
expect(commitB).not.toBeUndefined();
245245
expect(await ci.identifyMergeCommit(config.repo.trackingBranches[2], commitG)).toEqual(commitD);
246246
expect(await ci.identifyMergeCommit(config.repo.trackingBranches[2], commitE)).toEqual(commitC);

tests/ci-helper.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ function testQ(label: string, fn: AsyncFn) {
3232
});
3333
}
3434

35-
getConfig();
35+
const config = getConfig();
3636

3737
const eMailOptions = {
3838
smtpserver: new testSmtpServer(),
@@ -61,7 +61,7 @@ class TestCIHelper extends CIHelper {
6161
public addPRLabelsCalls: Array<[_: string, labels: string[]]>;
6262

6363
public constructor(workDir: string, debug = false, gggDir = ".") {
64-
super(workDir, debug, gggDir);
64+
super(workDir, config, debug, gggDir);
6565
this.testing = true;
6666
this.ghGlue = this.github;
6767

@@ -203,7 +203,7 @@ testQ("identify merge that integrated some commit", async () => {
203203
const commitD = await repo.merge("d", commitF);
204204
await repo.git(["update-ref", "refs/remotes/upstream/seen", commitD]);
205205

206-
const ci = new CIHelper(repo.workDir, true);
206+
const ci = new CIHelper(repo.workDir, config, true);
207207
expect(commitB).not.toBeUndefined();
208208
expect(await ci.identifyMergeCommit("seen", commitG)).toEqual(commitD);
209209
expect(await ci.identifyMergeCommit("seen", commitE)).toEqual(commitC);

0 commit comments

Comments
 (0)