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

fix: normalize whether build happens before lint/knip #1152

Merged
merged 4 commits into from
Dec 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .github/workflows/lint-knip.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ jobs:
steps:
- uses: actions/checkout@v4
- uses: ./.github/actions/prepare
- run: pnpm build || true
- run: pnpm lint:knip

name: Lint Knip
Expand Down
13 changes: 0 additions & 13 deletions script/__snapshots__/migrate-test-e2e.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -50,19 +50,6 @@ exports[`expected file changes > .eslintrc.cjs 1`] = `
{"
`;

exports[`expected file changes > .github/workflows/lint-knip.yml 1`] = `
"--- a/.github/workflows/lint-knip.yml
+++ b/.github/workflows/lint-knip.yml
@@ ... @@ jobs:
steps:
- uses: actions/checkout@v4
- uses: ./.github/actions/prepare
- - run: pnpm build || true
- run: pnpm lint:knip

name: Lint Knip"
`;

exports[`expected file changes > .github/workflows/test.yml 1`] = `
"--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
Expand Down
1 change: 0 additions & 1 deletion script/migrate-test-e2e.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ const filesExpectedToBeChanged = [
"package.json",
".eslintignore",
".eslintrc.cjs",
".github/workflows/lint-knip.yml",
".github/workflows/test.yml",
".gitignore",
".prettierignore",
Expand Down
13 changes: 8 additions & 5 deletions src/create/createWithOptions.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { $ } from "execa";

import { withSpinner, withSpinners } from "../shared/cli/spinners.js";
import { createCleanUpFilesCommands } from "../shared/createCleanUpFilesCommands.js";
import { doesRepositoryExist } from "../shared/doesRepositoryExist.js";
import { GitHubAndOptions } from "../shared/options/readOptions.js";
import { addToolAllContributors } from "../steps/addToolAllContributors.js";
Expand Down Expand Up @@ -37,11 +38,13 @@ export async function createWithOptions({ github, options }: GitHubAndOptions) {
finalizeDependencies(options),
);

await runCommands("Cleaning up files", [
"pnpm dedupe",
"pnpm lint --fix",
"pnpm format --write",
]);
await runCommands(
"Cleaning up files",
createCleanUpFilesCommands({
bin: !!options.bin,
dedupe: true,
}),
);
}

const sendToGitHub =
Expand Down
11 changes: 7 additions & 4 deletions src/initialize/initializeWithOptions.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { withSpinner, withSpinners } from "../shared/cli/spinners.js";
import { createCleanUpFilesCommands } from "../shared/createCleanUpFilesCommands.js";
import { GitHubAndOptions } from "../shared/options/readOptions.js";
import { addOwnerAsAllContributor } from "../steps/addOwnerAsAllContributor.js";
import { clearChangelog } from "../steps/clearChangelog.js";
Expand Down Expand Up @@ -60,8 +61,10 @@ export async function initializeWithOptions({
);
}

await runCommands("Cleaning up files", [
"pnpm lint --fix",
"pnpm format --write",
]);
await runCommands(
"Cleaning up files",
createCleanUpFilesCommands({
bin: !!options.bin,
}),
);
}
12 changes: 8 additions & 4 deletions src/migrate/migrateWithOptions.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { withSpinner, withSpinners } from "../shared/cli/spinners.js";
import { createCleanUpFilesCommands } from "../shared/createCleanUpFilesCommands.js";
import { GitHubAndOptions } from "../shared/options/readOptions.js";
import { clearUnnecessaryFiles } from "../steps/clearUnnecessaryFiles.js";
import { detectExistingContributors } from "../steps/detectExistingContributors.js";
Expand Down Expand Up @@ -60,8 +61,11 @@ export async function migrateWithOptions({
);
}

await runCommands("Cleaning up files", [
"pnpm lint --fix",
"pnpm format --write",
]);
await runCommands(
"Cleaning up files",
createCleanUpFilesCommands({
bin: !!options.bin,
dedupe: true,
}),
);
}
25 changes: 25 additions & 0 deletions src/shared/createCleanUpFilesCommands.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { describe, expect, it } from "vitest";

import { createCleanUpFilesCommands } from "./createCleanUpFilesCommands.js";

describe("createCleanUpFilesCommands", () => {
it("only lints and formats when no options are specified", () => {
const actual = createCleanUpFilesCommands({});

expect(actual).toEqual(["pnpm lint --fix", "pnpm format --write"]);
});

it("runs dedupe and build before it lints and formats when both options are specified", () => {
const actual = createCleanUpFilesCommands({
bin: true,
dedupe: true,
});

expect(actual).toEqual([
"pnpm dedupe",
"pnpm build || exit 0",
"pnpm lint --fix",
"pnpm format --write",
]);
});
});
18 changes: 18 additions & 0 deletions src/shared/createCleanUpFilesCommands.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
export interface CleanUpFilesOptions {
bin?: boolean;
dedupe?: boolean;
}

export function createCleanUpFilesCommands({
bin,
dedupe,
}: CleanUpFilesOptions) {
return [
// There's no need to dedupe when initializing from the fixed template
...(dedupe ? ["pnpm dedupe"] : []),
// n/no-missing-import rightfully reports on a missing the bin .js file
...(bin ? ["pnpm build || exit 0"] : []),
"pnpm lint --fix",
"pnpm format --write",
];
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const createOptions = (exclude: boolean) =>
({
access: "public",
base: "everything",
bin: exclude ? undefined : "./bin/index.js",
description: "Test description.",
directory: ".",
email: {
Expand Down Expand Up @@ -413,7 +414,6 @@ describe("createWorkflows", () => {
steps:
- uses: actions/checkout@v4
- uses: ./.github/actions/prepare
- run: pnpm build || true
- run: pnpm lint

name: Lint
Expand Down
2 changes: 1 addition & 1 deletion src/steps/writing/creation/dotGitHub/createWorkflows.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ export function createWorkflows(options: Options) {
}),
"lint.yml": createWorkflowFile({
name: "Lint",
runs: ["pnpm build || true", "pnpm lint"],
runs: [...(options.bin ? ["pnpm build || true"] : []), "pnpm lint"],
}),
...(!options.excludeLintKnip && {
"lint-knip.yml": createWorkflowFile({
Expand Down