Update TypeScript workflow to run only if TS-related samples change - #24691
Conversation
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".github/workflows/samples-typescript-typecheck.yaml">
<violation number="1" location=".github/workflows/samples-typescript-typecheck.yaml:4">
P3: Adding a push trigger alongside pull_request runs this workflow twice for the same commit on any in-repo feature branch that has an open PR. For fork-based PRs only pull_request fires, but for same-repo branches the duplication partially undercuts the PR's stated goal of reducing CI noise. Consider dropping the push trigger or restricting it to master.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| name: TypeScript clients type checks | ||
|
|
||
| on: | ||
| push: |
There was a problem hiding this comment.
P3: Adding a push trigger alongside pull_request runs this workflow twice for the same commit on any in-repo feature branch that has an open PR. For fork-based PRs only pull_request fires, but for same-repo branches the duplication partially undercuts the PR's stated goal of reducing CI noise. Consider dropping the push trigger or restricting it to master.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/samples-typescript-typecheck.yaml, line 4:
<comment>Adding a push trigger alongside pull_request runs this workflow twice for the same commit on any in-repo feature branch that has an open PR. For fork-based PRs only pull_request fires, but for same-repo branches the duplication partially undercuts the PR's stated goal of reducing CI noise. Consider dropping the push trigger or restricting it to master.</comment>
<file context>
@@ -1,9 +1,22 @@
name: TypeScript clients type checks
on:
+ push:
+ paths:
+ - samples/client/petstore/typescript*/**
</file context>
This reverts commit 29badc9.
There was a problem hiding this comment.
1 issue found and verified against the latest diff
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".github/workflows/samples-typescript-typecheck.yaml">
<violation number="1" location=".github/workflows/samples-typescript-typecheck.yaml:11">
P2: The new path filters may not cover every directory that `bin/ts-typecheck-all.sh` typechecks, so changing/adding a TypeScript sample that the script still processes will silently skip this workflow. In particular `samples/openapi3/client/petstore/typescript/**` is written without the `*` wildcard used everywhere else, so a suffixed directory such as `.../typescript-fetch/` or `.../typescript-angular/` under `samples/openapi3/client/petstore/` would not trigger the job even though the typecheck script iterates over those samples. Align the pattern with the others (`typescript*/**`) and, ideally, confirm the filter list matches the set of samples the script actually checks (including any `push`-only regression against the previous broad `samples/**` coverage for pull_request).</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| - samples/client/petstore/typescript*/** | ||
| - samples/client/others/typescript*/** | ||
| - samples/client/echo_api/typescript*/** | ||
| - samples/openapi3/client/petstore/typescript/** |
There was a problem hiding this comment.
P2: The new path filters may not cover every directory that bin/ts-typecheck-all.sh typechecks, so changing/adding a TypeScript sample that the script still processes will silently skip this workflow. In particular samples/openapi3/client/petstore/typescript/** is written without the * wildcard used everywhere else, so a suffixed directory such as .../typescript-fetch/ or .../typescript-angular/ under samples/openapi3/client/petstore/ would not trigger the job even though the typecheck script iterates over those samples. Align the pattern with the others (typescript*/**) and, ideally, confirm the filter list matches the set of samples the script actually checks (including any push-only regression against the previous broad samples/** coverage for pull_request).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/samples-typescript-typecheck.yaml, line 9:
<comment>The new path filters may not cover every directory that `bin/ts-typecheck-all.sh` typechecks, so changing/adding a TypeScript sample that the script still processes will silently skip this workflow. In particular `samples/openapi3/client/petstore/typescript/**` is written without the `*` wildcard used everywhere else, so a suffixed directory such as `.../typescript-fetch/` or `.../typescript-angular/` under `samples/openapi3/client/petstore/` would not trigger the job even though the typecheck script iterates over those samples. Align the pattern with the others (`typescript*/**`) and, ideally, confirm the filter list matches the set of samples the script actually checks (including any `push`-only regression against the previous broad `samples/**` coverage for pull_request).</comment>
<file context>
@@ -1,9 +1,22 @@
+ - samples/client/petstore/typescript*/**
+ - samples/client/others/typescript*/**
+ - samples/client/echo_api/typescript*/**
+ - samples/openapi3/client/petstore/typescript/**
+ - samples/server/petstore/typescript-nestjs-server/**
+ - bin/ts-typecheck-all.sh
</file context>
| - samples/openapi3/client/petstore/typescript/** | |
| - samples/openapi3/client/petstore/typescript*/** |
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".github/workflows/samples-typescript-typecheck.yaml">
<violation number="1" location=".github/workflows/samples-typescript-typecheck.yaml:11">
P2: The `samples/openapi3/client/petstore/typescript/**` path pattern omits the `*` after `typescript` that the sibling client patterns use, so any openapi3 TypeScript sample in a `typescript-*` subdirectory would not trigger this workflow. Use `typescript*/**` for consistency, or confirm no such openapi3 variants exist.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| - samples/client/petstore/typescript*/** | ||
| - samples/client/others/typescript*/** | ||
| - samples/client/echo_api/typescript*/** | ||
| - samples/openapi3/client/petstore/typescript/** |
There was a problem hiding this comment.
P2: The samples/openapi3/client/petstore/typescript/** path pattern omits the * after typescript that the sibling client patterns use, so any openapi3 TypeScript sample in a typescript-* subdirectory would not trigger this workflow. Use typescript*/** for consistency, or confirm no such openapi3 variants exist.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/samples-typescript-typecheck.yaml, line 11:
<comment>The `samples/openapi3/client/petstore/typescript/**` path pattern omits the `*` after `typescript` that the sibling client patterns use, so any openapi3 TypeScript sample in a `typescript-*` subdirectory would not trigger this workflow. Use `typescript*/**` for consistency, or confirm no such openapi3 variants exist.</comment>
<file context>
@@ -1,9 +1,24 @@
+ - samples/client/petstore/typescript*/**
+ - samples/client/others/typescript*/**
+ - samples/client/echo_api/typescript*/**
+ - samples/openapi3/client/petstore/typescript/**
+ - samples/server/petstore/typescript-nestjs-server/**
+ - bin/ts-typecheck-all.sh
</file context>
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".github/workflows/samples-typescript-typecheck.yaml">
<violation number="1" location=".github/workflows/samples-typescript-typecheck.yaml:8">
P3: The 7-path filter list is duplicated verbatim between the `push` and `pull_request` blocks, and it must stay in sync with both the added `paths` allowlist and the directories `bin/ts-typecheck-all.sh` iterates. Any future TypeScript sample added outside these globs (or a path fix applied to only one block) silently skips the typecheck. Consider hoisting the shared list (e.g. via a `yaml` anchor or a composite action) so it is maintained in one place.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| branches: | ||
| - master | ||
| paths: | ||
| - samples/client/petstore/typescript*/** |
There was a problem hiding this comment.
P3: The 7-path filter list is duplicated verbatim between the push and pull_request blocks, and it must stay in sync with both the added paths allowlist and the directories bin/ts-typecheck-all.sh iterates. Any future TypeScript sample added outside these globs (or a path fix applied to only one block) silently skips the typecheck. Consider hoisting the shared list (e.g. via a yaml anchor or a composite action) so it is maintained in one place.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/samples-typescript-typecheck.yaml, line 8:
<comment>The 7-path filter list is duplicated verbatim between the `push` and `pull_request` blocks, and it must stay in sync with both the added `paths` allowlist and the directories `bin/ts-typecheck-all.sh` iterates. Any future TypeScript sample added outside these globs (or a path fix applied to only one block) silently skips the typecheck. Consider hoisting the shared list (e.g. via a `yaml` anchor or a composite action) so it is maintained in one place.</comment>
<file context>
@@ -1,9 +1,24 @@
+ branches:
+ - master
+ paths:
+ - samples/client/petstore/typescript*/**
+ - samples/client/others/typescript*/**
+ - samples/client/echo_api/typescript*/**
</file context>
PR checklist
Commit all changed files.
This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
These must match the expectations made by your contribution.
You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example
./bin/generate-samples.sh bin/configs/java*.IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
Summary by cubic
Limits the TypeScript samples type-check workflow to TypeScript-only changes and adds a push trigger on
master. Previously it ran on any PR change undersamplesand did not run on push.samples/client/petstore/typescript*/**,samples/client/others/typescript*/**,samples/client/echo_api/typescript*/**,samples/openapi3/client/petstore*/typescript/**,samples/server/petstore/typescript-nestjs-server/**,bin/ts-typecheck-all.sh, or.github/workflows/samples-typescript-typecheck.yaml.masteronly with the same path filters.Written for commit ca8dd40. Summary will update on new commits.