Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: drop dependency on gh #1000

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ It includes options not just for building and testing but also GitHub repository

First make sure you have the following installed:

- [GitHub CLI](https://cli.github.com) _(you'll need to be logged in)_
- [Node.js](https://nodejs.org)
- [pnpm](https://pnpm.io)
- _(optional, but helpful)_ [GitHub CLI](https://cli.github.com) _(you'll need to be logged in)_

Then in an existing repository or in your directory where you'd like to make a new repository:

Expand Down
5 changes: 4 additions & 1 deletion src/create/createWithOptions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,10 @@ describe("createWithOptions", () => {
};

await createWithOptions({ github, options });
expect(addToolAllContributors).toHaveBeenCalledWith(options);
expect(addToolAllContributors).toHaveBeenCalledWith(
github.octokit,
options,
);
});

it("does not call addToolAllContributors when excludeAllContributors is true", async () => {
Expand Down
2 changes: 1 addition & 1 deletion src/create/createWithOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export async function createWithOptions({ github, options }: GitHubAndOptions) {

if (!options.excludeAllContributors && !options.skipAllContributorsApi) {
await withSpinner("Adding contributors to table", async () => {
await addToolAllContributors(options);
await addToolAllContributors(github?.octokit, options);
});
}

Expand Down
2 changes: 1 addition & 1 deletion src/initialize/initializeWithOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export async function initializeWithOptions({

if (!options.excludeAllContributors) {
await withSpinner("Updating existing contributor details", async () => {
await addOwnerAsAllContributor(options);
await addOwnerAsAllContributor(github?.octokit, options);
});
}

Expand Down
41 changes: 23 additions & 18 deletions src/shared/getGitHubUserAsAllContributor.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import chalk from "chalk";
import { Octokit } from "octokit";
import { MockInstance, beforeEach, describe, expect, it, vi } from "vitest";

import { getGitHubUserAsAllContributor } from "./getGitHubUserAsAllContributor.js";
Expand All @@ -13,6 +14,11 @@ vi.mock("execa", () => ({

let mockConsoleWarn: MockInstance;

const createMockOctokit = (getAuthenticated: MockInstance = vi.fn()) =>
({
rest: { users: { getAuthenticated } },
}) as unknown as Octokit;

const owner = "TestOwner";

describe("getGitHubUserAsAllContributor", () => {
Expand All @@ -23,7 +29,8 @@ describe("getGitHubUserAsAllContributor", () => {
});

it("defaults to owner with a log when options.offline is true", async () => {
const actual = await getGitHubUserAsAllContributor({
const octokit = createMockOctokit();
const actual = await getGitHubUserAsAllContributor(octokit, {
offline: true,
owner,
});
Expand All @@ -36,23 +43,24 @@ describe("getGitHubUserAsAllContributor", () => {
);
});

it("defaults to owner without a log when octokit is undefined", async () => {
const actual = await getGitHubUserAsAllContributor(undefined, { owner });

expect(actual).toEqual(owner);
expect(mockConsoleWarn).not.toHaveBeenCalled();
});

it("uses the user from gh api user when it succeeds", async () => {
const login = "gh-api-user";
const octokit = createMockOctokit(
vi.fn().mockResolvedValue({ data: { login } }),
);

mock$.mockResolvedValueOnce({
stdout: JSON.stringify({ login }),
});

await getGitHubUserAsAllContributor({ owner });
await getGitHubUserAsAllContributor(octokit, { owner });

expect(mockConsoleWarn).not.toHaveBeenCalled();
expect(mock$.mock.calls).toMatchInlineSnapshot(`
[
[
[
"gh api user",
],
],
[
[
"npx -y all-contributors-cli@6.25 add ",
Expand All @@ -67,9 +75,11 @@ describe("getGitHubUserAsAllContributor", () => {
});

it("defaults the user to the owner when gh api user fails", async () => {
mock$.mockRejectedValueOnce({});
const octokit = createMockOctokit(
vi.fn().mockRejectedValue(new Error("Oh no!")),
);

await getGitHubUserAsAllContributor({ owner });
await getGitHubUserAsAllContributor(octokit, { owner });

expect(mockConsoleWarn).toHaveBeenCalledWith(
chalk.gray(
Expand All @@ -78,11 +88,6 @@ describe("getGitHubUserAsAllContributor", () => {
);
expect(mock$.mock.calls).toMatchInlineSnapshot(`
[
[
[
"gh api user",
],
],
[
[
"npx -y all-contributors-cli@6.25 add ",
Expand Down
26 changes: 14 additions & 12 deletions src/shared/getGitHubUserAsAllContributor.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
import chalk from "chalk";
import { $ } from "execa";
import { Octokit } from "octokit";

import { Options } from "./types.js";

interface GhUserOutput {
login: string;
}

export async function getGitHubUserAsAllContributor(
octokit: Octokit | undefined,
options: Pick<Options, "offline" | "owner">,
) {
if (options.offline) {
Expand All @@ -21,14 +19,18 @@ export async function getGitHubUserAsAllContributor(

let user: string;

try {
user = (JSON.parse((await $`gh api user`).stdout) as GhUserOutput).login;
} catch {
console.warn(
chalk.gray(
`Couldn't authenticate GitHub user, falling back to the provided owner name '${options.owner}'.`,
),
);
if (octokit) {
try {
user = (await octokit.rest.users.getAuthenticated()).data.login;
} catch {
console.warn(
chalk.gray(
`Couldn't authenticate GitHub user, falling back to the provided owner name '${options.owner}'.`,
),
);
user = options.owner;
}
} else {
user = options.owner;
}

Expand Down
2 changes: 1 addition & 1 deletion src/shared/options/getGitHub.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ describe("getOctokit", () => {
});

await expect(getGitHub).rejects.toMatchInlineSnapshot(
"[Error: GitHub authentication failed.]",
`[Error: Couldn't authenticate with GitHub. Either log in with \`gh auth login\` (https://cli.github.com) or set a GH_TOKEN environment variable.]`,
);
});

Expand Down
7 changes: 4 additions & 3 deletions src/shared/options/getGitHub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,10 @@ export async function getGitHub(): Promise<GitHub | undefined> {
const auth = await getGitHubAuthToken();

if (!auth.succeeded) {
throw new Error("GitHub authentication failed.", {
cause: auth.error,
});
throw new Error(
"Couldn't authenticate with GitHub. Either log in with `gh auth login` (https://cli.github.com) or set a GH_TOKEN environment variable.",
{ cause: auth.error },
);
}

const octokit = new Octokit({ auth: auth.token });
Expand Down
8 changes: 4 additions & 4 deletions src/steps/addOwnerAsAllContributor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ describe("addOwnerAsAllContributor", () => {
mockReadFileAsJson.mockResolvedValue("invalid");

await expect(async () => {
await addOwnerAsAllContributor({ owner: mockOwner });
await addOwnerAsAllContributor(undefined, { owner: mockOwner });
}).rejects.toMatchInlineSnapshot(
'[Error: Invalid .all-contributorsrc: "invalid"]',
);
Expand All @@ -54,7 +54,7 @@ describe("addOwnerAsAllContributor", () => {
mockReadFileAsJson.mockResolvedValue({});

await expect(async () => {
await addOwnerAsAllContributor({ owner: mockOwner });
await addOwnerAsAllContributor(undefined, { owner: mockOwner });
}).rejects.toMatchInlineSnapshot(
"[Error: Invalid .all-contributorsrc: {}]",
);
Expand All @@ -68,7 +68,7 @@ describe("addOwnerAsAllContributor", () => {
contributors: [],
});

await addOwnerAsAllContributor({ owner: mockOwner });
await addOwnerAsAllContributor(undefined, { owner: mockOwner });

expect(mockWriteFile).toHaveBeenCalledWith(
"./.all-contributorsrc",
Expand All @@ -89,7 +89,7 @@ describe("addOwnerAsAllContributor", () => {
],
});

await addOwnerAsAllContributor({ owner: mockOwner });
await addOwnerAsAllContributor(undefined, { owner: mockOwner });

expect(mockWriteFile).toHaveBeenCalledWith(
"./.all-contributorsrc",
Expand Down
4 changes: 3 additions & 1 deletion src/steps/addOwnerAsAllContributor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@
import { readFileAsJson } from "../shared/readFileAsJson.js";
import { AllContributorsData, Options } from "../shared/types.js";
import { formatJson } from "./writing/creation/formatters/formatJson.js";
import { Octokit } from "octokit";

Check failure on line 7 in src/steps/addOwnerAsAllContributor.ts

View workflow job for this annotation

GitHub Actions / lint

Expected "octokit" to come before "./writing/creation/formatters/formatJson.js"

export async function addOwnerAsAllContributor(
octokit: Octokit | undefined,
options: Pick<Options, "offline" | "owner">,
) {
const user = await getGitHubUserAsAllContributor(options);
const user = await getGitHubUserAsAllContributor(octokit, options);

const existingContributors = (await readFileAsJson(
"./.all-contributorsrc",
Expand Down
4 changes: 2 additions & 2 deletions src/steps/addToolAllContributors.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,15 @@ describe("addToolAllContributors", () => {
it("adds JoshuaKGoldberg when that is not the current github user", async () => {
mockGetGitHubUserAsAllContributor.mockResolvedValue("JoshuaKGoldberg");

await addToolAllContributors({ owner: "owner" });
await addToolAllContributors(undefined, { owner: "owner" });

expect(mock$).not.toHaveBeenCalled();
});

it("does not add JoshuaKGoldberg when that not the current github user", async () => {
mockGetGitHubUserAsAllContributor.mockResolvedValue("other");

await addToolAllContributors({ owner: "owner" });
await addToolAllContributors(undefined, { owner: "owner" });

expect(mock$).toHaveBeenCalledWith([
`npx -y all-contributors-cli add JoshuaKGoldberg tool`,
Expand Down
4 changes: 3 additions & 1 deletion src/steps/addToolAllContributors.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
import { $ } from "execa";
import { Octokit } from "octokit";

import { getGitHubUserAsAllContributor } from "../shared/getGitHubUserAsAllContributor.js";
import { Options } from "../shared/types.js";

export async function addToolAllContributors(
octokit: Octokit | undefined,
options: Pick<Options, "offline" | "owner">,
) {
const login = await getGitHubUserAsAllContributor(options);
const login = await getGitHubUserAsAllContributor(octokit, options);

if (login !== "JoshuaKGoldberg") {
await $`npx -y all-contributors-cli add JoshuaKGoldberg tool`;
Expand Down
2 changes: 1 addition & 1 deletion src/steps/initializeGitHubRepository/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,5 @@ export async function initializeGitHubRepository(
await initializeGitRemote(options);
await initializeRepositorySettings(octokit, options);
await initializeBranchProtectionSettings(octokit, options);
await initializeRepositoryLabels();
await initializeRepositoryLabels(octokit, options);
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ const aliases = new Map([

export interface GhLabelData {
color: string;
description: string;
description: null | string;
name: string;
}

Expand Down
Loading
Loading