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.

Sign up for GitHub

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: Compile each project individually #969

Merged
merged 2 commits into from
May 3, 2024
Merged

fix: Compile each project individually #969

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b721fbb
ci: Compile each package individually
akash1810 May 3, 2024
83ab334
fix: Add explicit return type to `createPullRequest`
akash1810 May 3, 2024
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
3 changes: 2 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
"test": "npm run test --workspaces --if-present",
"test-update": "jest -u",
"synth": "npm run synth --workspace=cdk",
"typecheck": "tsc",
"pretypecheck": "npm -w common run typecheck",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pretypecheck will always run before typecheck see https://docs.npmjs.com/cli/v10/using-npm/scripts#pre--post-scripts. We need to compile the shared package first, as other projects depend on it, and it doesn't look like TypeScript understands this dependency.

"typecheck": "npm run typecheck --workspaces",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its worth noting we were previously compiling each project individually (see #520). The difference now though is we're not using the --if-present flag, so the concerns mentioned in #520 do not hold.

"build": "npm run build --workspaces --if-present",
"lint": "eslint packages/** --ext .ts --no-error-on-unmatched-pattern"
},
Expand Down
3 changes: 2 additions & 1 deletion packages/best-practices/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
"description": "A markdown file of best practices generated by a script from a definitions file",
"type": "module",
"scripts": {
"generate": "tsx src/index.ts"
"generate": "tsx src/index.ts",
"typecheck": "tsc --noEmit"
},
"devDependencies": {
"markdown-table": "^3.0.3"
Expand Down
3 changes: 2 additions & 1 deletion packages/cdk/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
"scripts": {
"test": "jest --detectOpenHandles --config ../../jest.config.js --selectProjects cdk",
"synth": "cdk synth --path-metadata false --version-reporting false",
"diff:code": "cdk diff --path-metadata false --version-reporting false --profile deployTools ServiceCatalogue-CODE"
"diff:code": "cdk diff --path-metadata false --version-reporting false --profile deployTools ServiceCatalogue-CODE",
"typecheck": "tsc --noEmit"
},
"devDependencies": {
"@guardian/cdk": "58.0.0",
Expand Down
3 changes: 2 additions & 1 deletion packages/cli/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
"name": "cli",
"version": "0.0.0",
"scripts": {
"start": "tsx src/index.ts"
"start": "tsx src/index.ts",
"typecheck": "tsc --noEmit"
},
"type": "module",
"devDependencies": {
Expand Down
3 changes: 2 additions & 1 deletion packages/cloudbuster/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
"name": "cloudbuster",
"version": "1.0.0",
"scripts": {
"start": "APP=cloudbuster tsx src/run-locally.ts"
"start": "APP=cloudbuster tsx src/run-locally.ts",
"typecheck": "tsc --noEmit"
}
}
3 changes: 2 additions & 1 deletion packages/common/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
"type": "module",
"scripts": {
"postinstall": "prisma generate",
"test": "jest --detectOpenHandles --config ../../jest.config.js --selectProjects common"
"test": "jest --detectOpenHandles --config ../../jest.config.js --selectProjects common",
"typecheck": "tsc"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When compiling the common package, write the resulting files to disk (i.e. no --noEmit flag), so that it can be used by the other projects during their compile.

},
"dependencies": {
"@aws-sdk/client-secrets-manager": "^3.568.0",
Expand Down
30 changes: 19 additions & 11 deletions packages/common/src/pull-requests.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,18 +23,24 @@ export function generateBranchName(prefix: string) {
return `${prefix}-${randomBytes(8).toString('hex')}`;
}

/**
* Creates or updates a pull request, and return its URL.
* On error, an exception is thrown, or undefined is returned.
*/
export async function createPullRequest(
octokit: Octokit,
{
props: CreatePullRequestOptions,
Copy link
Member Author

@akash1810 akash1810 May 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change moves the object destructing out of the function signature, in an attempt to improve readability. This is totally subjective though!

): Promise<string | undefined> {
const {
repoName,
title,
body,
branchName,
baseBranch = 'main',
changes,
}: CreatePullRequestOptions,
) {
return await composeCreatePullRequest(octokit, {
} = props;

const response = await composeCreatePullRequest(octokit, {
owner: 'guardian',
repo: repoName,
title,
Expand All @@ -46,6 +52,8 @@ export async function createPullRequest(
files,
})),
});

return response?.data.html_url;
Copy link
Member Author

@akash1810 akash1810 May 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the main change here, and resolves the compilation error in the PR description.

The return type of composeCreatePullRequest is a bit tricky to infer. As we're (currently) only using the html_url property, update createPullRequest to return that, and therefore the return type of createPullRequest becomes Promise<string | undefined>.

}

type PullRequestParameters =
Expand Down Expand Up @@ -99,7 +107,7 @@ export async function createPrAndAddToProject(
);

if (!existingPullRequest) {
const response = await createPullRequest(octokit, {
const pullRequestUrl = await createPullRequest(octokit, {
repoName,
title: prTitle,
body: prBody,
Expand All @@ -113,12 +121,12 @@ export async function createPrAndAddToProject(
},
],
});
console.log(
'Pull request successfully created:',
response?.data.html_url,
);
await addPrToProject(stage, repoName, boardNumber, author);
console.log('Updated project board');

if (pullRequestUrl) {
console.log('Pull request successfully created:', pullRequestUrl);
await addPrToProject(stage, repoName, boardNumber, author);
console.log('Updated project board');
}
} else {
console.log(
`Existing pull request found. Skipping creating a new one.`,
Expand Down
3 changes: 2 additions & 1 deletion packages/data-audit/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
"test": "echo \"Error: no test specified\"",
"start": "APP=data-audit tsx src/run-locally.ts",
"prebuild": "rm -rf dist",
"build": "esbuild src/index.ts --bundle --platform=node --target=node20 --outdir=dist --external:@aws-sdk --external:@prisma/client --external:prisma"
"build": "esbuild src/index.ts --bundle --platform=node --target=node20 --outdir=dist --external:@aws-sdk --external:@prisma/client --external:prisma",
"typecheck": "tsc --noEmit"
},
"dependencies": {
"@aws-sdk/client-lambda": "^3.568.0",
Expand Down
3 changes: 2 additions & 1 deletion packages/dependency-graph-integrator/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
"scripts": {
"build": "esbuild src/index.ts --bundle --platform=node --target=node20 --outdir=dist --external:@aws-sdk",
"start": "tsx src/run-locally.ts",
"test": "jest --detectOpenHandles --config ../../jest.config.js --selectProjects dependency-graph-integrator"
"test": "jest --detectOpenHandles --config ../../jest.config.js --selectProjects dependency-graph-integrator",
"typecheck": "tsc --noEmit"
},
"dependencies": {
"@aws-sdk/client-sns": "^3.568.0",
Expand Down
3 changes: 2 additions & 1 deletion packages/dev-environment/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
"version": "0.0.0",
"scripts": {
"start": "./script/start",
"test": "find . | egrep '.yml|.yaml' | xargs yamllint"
"test": "find . | egrep '.yml|.yaml' | xargs yamllint",
"typecheck": "echo 'Nothing to typecheck'"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This package has no TypeScript code, so there's nothing to do.

},
"devDependencies": {
"yaml-lint": "^1.7.0"
Expand Down
3 changes: 2 additions & 1 deletion packages/diagrams/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
"description": "Diagrams as code",
"type": "module",
"scripts": {
"generate": "mmdc -i diagram.mmd -o output.svg "
"generate": "mmdc -i diagram.mmd -o output.svg ",
"typecheck": "echo 'Nothing to typecheck'"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This package has no TypeScript code, so there's nothing to do.

},
"devDependencies": {
"@mermaid-js/mermaid-cli": "10.8.0"
Expand Down
3 changes: 2 additions & 1 deletion packages/github-actions-usage/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
"test": "node --import tsx --test **/*.test.ts",
"start": "APP=github-actions-usage tsx src/run-locally.ts",
"prebuild": "rm -rf dist",
"build": "esbuild src/index.ts --bundle --platform=node --target=node20 --outdir=dist --external:@prisma/client --external:prisma"
"build": "esbuild src/index.ts --bundle --platform=node --target=node20 --outdir=dist --external:@prisma/client --external:prisma",
"typecheck": "tsc --noEmit"
},
"type": "module",
"dependencies": {
Expand Down
3 changes: 2 additions & 1 deletion packages/interactive-monitor/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
"scripts": {
"build": "esbuild src/index.ts --bundle --platform=node --target=node20 --outdir=dist --external:@aws-sdk",
"test": "jest --detectOpenHandles --config ../../jest.config.js --selectProjects interactive-monitor",
"start": "tsx src/run-locally.ts"
"start": "tsx src/run-locally.ts",
"typecheck": "tsc --noEmit"
},
"author": "guardian",
"dependencies": {
Expand Down
3 changes: 2 additions & 1 deletion packages/repocop/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@
"scripts": {
"build": "esbuild src/index.ts --bundle --platform=node --target=node20 --outdir=dist --external:@aws-sdk --external:@prisma/client --external:prisma",
"start": "APP=repocop tsx src/run-locally.ts",
"test": "jest --detectOpenHandles --config ../../jest.config.js --selectProjects repocop"
"test": "jest --detectOpenHandles --config ../../jest.config.js --selectProjects repocop",
"typecheck": "tsc --noEmit"
},
"dependencies": {
"@aws-sdk/client-cloudwatch": "^3.568.0",
Expand Down
3 changes: 2 additions & 1 deletion packages/snyk-integrator/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
"scripts": {
"build": "esbuild src/index.ts --bundle --platform=node --target=node20 --outdir=dist --external:@aws-sdk",
"start": "APP=snyk-integrator tsx src/run-locally.ts",
"test": "jest --detectOpenHandles --config ../../jest.config.js --selectProjects snyk-integrator"
"test": "jest --detectOpenHandles --config ../../jest.config.js --selectProjects snyk-integrator",
"typecheck": "tsc --noEmit"
},
"dependencies": {
"@aws-sdk/client-sns": "^3.568.0",
Expand Down
1 change: 0 additions & 1 deletion tsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
{
"extends": "@guardian/tsconfig/tsconfig.json",
"compilerOptions": {
"noEmit": true,
"module": "commonjs",
"esModuleInterop": true,
"baseUrl": ".",
Expand Down
Loading