diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 45313ab8828a..05752938c467 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -1,8 +1,8 @@ -## What this PR solves / how to test +Fixes #[insert GH or internal issue link(s)]. -Fixes #[insert GH or internal issue number(s)]. +_Describe your change..._ -## Author has addressed the following +--- - Tests - [ ] TODO (before merge) @@ -12,10 +12,6 @@ Fixes #[insert GH or internal issue number(s)]. - [ ] I don't know - [ ] Required - [ ] Not required because: -- Changeset ([Changeset guidelines](https://github.com/cloudflare/workers-sdk/blob/main/CONTRIBUTING.md#changesets)) - - [ ] TODO (before merge) - - [ ] Changeset included - - [ ] Changeset not necessary because: - Public documentation - [ ] TODO (before merge) - [ ] Cloudflare docs PR(s): @@ -25,9 +21,3 @@ Fixes #[insert GH or internal issue number(s)]. Have you read our [Contributing guide](https://github.com/cloudflare/workers-sdk/blob/main/CONTRIBUTING.md)? In particular, for non-trivial changes, please always engage on the issue or create a discussion or feature request issue first before writing your code. --> - - diff --git a/.github/workflows/validate-pr-description.yml b/.github/workflows/validate-pr-description.yml index 6381bc159ff1..2c51d18ccd43 100644 --- a/.github/workflows/validate-pr-description.yml +++ b/.github/workflows/validate-pr-description.yml @@ -26,6 +26,11 @@ jobs: everything_but_markdown: - '!**/*.md' + - uses: jitterbit/get-changed-files@v1 + id: files + with: + format: "json" + - name: Install Dependencies if: steps.changes.outputs.everything_but_markdown == 'true' uses: ./.github/actions/install-dependencies @@ -41,3 +46,4 @@ jobs: TITLE: ${{ github.event.pull_request.title }} BODY: ${{ github.event.pull_request.body }} LABELS: ${{ toJson(github.event.pull_request.labels.*.name) }} + FILES: ${{ steps.files.outputs.all }} diff --git a/tools/deployments/__tests__/validate-pr-description.test.ts b/tools/deployments/__tests__/validate-pr-description.test.ts index 6b37399362dc..46a6e552548c 100644 --- a/tools/deployments/__tests__/validate-pr-description.test.ts +++ b/tools/deployments/__tests__/validate-pr-description.test.ts @@ -4,7 +4,7 @@ import { validateDescription } from "../validate-pr-description"; describe("validateDescription()", () => { it("should skip validation with the `skip-pr-description-validation` label", () => { expect( - validateDescription("", "", '["skip-pr-description-validation"]') + validateDescription("", "", '["skip-pr-description-validation"]', "[]") ).toHaveLength(0); }); @@ -26,15 +26,12 @@ Fixes #[insert GH or internal issue number(s)]. - [ ] I don't know - [ ] Required - [ ] Not required because: -- Changeset ([Changeset guidelines](https://github.com/cloudflare/workers-sdk/blob/main/CONTRIBUTING.md#changesets)) - - [ ] TODO (before merge) - - [ ] Changeset included - - [ ] Changeset not necessary because: - Public documentation - [x] TODO (before merge) - [ ] Cloudflare docs PR(s): - [ ] Documentation not necessary because: `, + "[]", "[]" ) ).toMatchInlineSnapshot(` @@ -42,12 +39,41 @@ Fixes #[insert GH or internal issue number(s)]. "All TODO checkboxes in your PR description must be unchecked before merging", "Your PR must include tests, or provide justification for why no tests are required", "Your PR must run E2E tests, or provide justification for why running them is not required", - "Your PR must include a changeset, or provide justification for why no changesets are required", + "Your PR doesn't include a changeset. Either include one (following the instructions in CONTRIBUTING.md) or add the 'no-changeset-required' label to bypass this check. Most PRs should have a changeset, so only bypass this check if you're sure that your change doesn't need one: see https://github.com/cloudflare/workers-sdk/blob/main/CONTRIBUTING.md#changesets for more details.", "Your PR must include documentation (in the form of a link to a Cloudflare Docs issue or PR), or provide justification for why no documentation is required", ] `); }); + it("should bypass changesets check with label", () => { + expect( + validateDescription( + "", + `## What this PR solves / how to test + +Fixes #[insert GH or internal issue number(s)]. + +## Author has addressed the following + +- Tests + - [ ] TODO (before merge) + - [x] Tests included + - [ ] Tests not necessary because: +- E2E Tests CI Job required? (Use "e2e" label or ask maintainer to run separately) + - [ ] I don't know + - [ ] Required + - [x] Not required because: test +- Public documentation + - [ ] TODO (before merge) + - [ ] Cloudflare docs PR(s): + - [x] Documentation not necessary because: test +`, + '["no-changeset-required"]', + "[]" + ) + ).toHaveLength(0); + }); + it("should accept everything included", () => { expect( validateDescription( @@ -75,7 +101,8 @@ Fixes [AA-000](https://jira.cfdata.org/browse/AA-000). - [x] Cloudflare docs PR(s): https://github.com/cloudflare/cloudflare-docs/pull/123 - [ ] Documentation not necessary because: `, - "[]" + "[]", + '[".changeset/hello-world.md"]' ) ).toHaveLength(0); }); @@ -107,7 +134,8 @@ Fixes [AA-000](https://jira.cfdata.org/browse/AA-000). - [x] Cloudflare docs PR(s): https://github.com/cloudflare/cloudflare-docs/pull/123 - [ ] Documentation not necessary because: `, - "[]" + "[]", + '[".changeset/hello-world.md"]' ) ).toMatchInlineSnapshot(` [ @@ -144,7 +172,8 @@ Fixes [AA-000](https://jira.cfdata.org/browse/AA-000). - [x] Cloudflare docs PR(s): https://github.com/cloudflare/cloudflare-docs/pull/123 - [ ] Documentation not necessary because: `, - "[]" + "[]", + '[".changeset/hello-world.md"]' ) ).toMatchInlineSnapshot(` [ @@ -180,7 +209,8 @@ Fixes [AA-000](https://jira.cfdata.org/browse/AA-000). - [x] Cloudflare docs PR(s): https://github.com/cloudflare/cloudflare-docs/pull/123 - [ ] Documentation not necessary because: `, - '["e2e"]' + '["e2e"]', + '[".changeset/hello-world.md"]' ) ).toHaveLength(0); }); @@ -212,7 +242,8 @@ Fixes [AA-000](https://jira.cfdata.org/browse/AA-000). - [X] Cloudflare docs PR(s): https://github.com/cloudflare/cloudflare-docs/pull/123 - [ ] Documentation not necessary because: `, - '["e2e"]' + '["e2e"]', + '[".changeset/hello-world.md"]' ) ).toHaveLength(0); }); diff --git a/tools/deployments/validate-pr-description.ts b/tools/deployments/validate-pr-description.ts index 48bd48535c62..ee830aaad0fc 100644 --- a/tools/deployments/validate-pr-description.ts +++ b/tools/deployments/validate-pr-description.ts @@ -3,7 +3,8 @@ if (require.main === module) { const errors = validateDescription( process.env.TITLE as string, process.env.BODY as string, - process.env.LABELS as string + process.env.LABELS as string, + process.env.FILES as string ); if (errors.length > 0) { console.error("Validation errors in PR description:"); @@ -17,12 +18,14 @@ if (require.main === module) { export function validateDescription( title: string, body: string, - labels: string + labels: string, + changedFilesJson: string ) { const errors: string[] = []; console.log("PR:", title); - const parsedLabels = JSON.parse(labels); + + const parsedLabels = JSON.parse(labels) as string[]; if (parsedLabels.includes("skip-pr-description-validation")) { console.log( @@ -71,14 +74,14 @@ export function validateDescription( ); } - if ( - !( - /- \[x\] Changeset included/i.test(body) || - /- \[x\] Changeset not necessary because: .+/i.test(body) - ) - ) { + const changedFiles = JSON.parse(changedFilesJson) as string[]; + const changesetIncluded = changedFiles.some((f) => + f.startsWith(".changeset/") + ); + + if (!changesetIncluded && !parsedLabels.includes("no-changeset-required")) { errors.push( - "Your PR must include a changeset, or provide justification for why no changesets are required" + "Your PR doesn't include a changeset. Either include one (following the instructions in CONTRIBUTING.md) or add the 'no-changeset-required' label to bypass this check. Most PRs should have a changeset, so only bypass this check if you're sure that your change doesn't need one: see https://github.com/cloudflare/workers-sdk/blob/main/CONTRIBUTING.md#changesets for more details." ); }