Skip to content

Commit 9176f4e

Browse files
committed
Address TypeScript SDK docs review feedback
Keeps selective-checks accurate for the new ts-sdk docs build (tsconfig.json and package.json changes now trigger it, and docs-only package.json/lock changes no longer trigger the unrelated supervisor-schema check), fixes the alphabetical ordering of the new run-ts-sdk-docs row, simplifies the @msgpack/msgpack path mapping to resolve through its package.json instead of a hardcoded internal path, wraps the long tsconfig.json comment, and aligns the README's cancellation section wording with task.ts's TSDoc.
1 parent d189da7 commit 9176f4e

5 files changed

Lines changed: 49 additions & 7 deletions

File tree

dev/breeze/doc/ci/04_selective_checks.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -595,8 +595,8 @@ GitHub Actions to pass the list of parameters to a command to execute
595595
| run-mypy | Whether mypy check is supposed to run in this build | true | |
596596
| run-system-tests | Whether system tests should be run ("true"/"false") | true | |
597597
| run-task-sdk-tests | Whether Task SDK tests should be run ("true"/"false") | true | |
598-
| run-ts-sdk-e2e-tests | Whether TypeScript SDK e2e tests should be run — on `ts-sdk/`, TS e2e test, or Node coordinator changes ("true"/"false") | true | |
599598
| run-ts-sdk-docs | Whether the TypeScript SDK API reference should be built — on `ts-sdk/docs/` or `ts-sdk/src/` changes, including Markdown ("true"/"false") | true | |
599+
| run-ts-sdk-e2e-tests | Whether TypeScript SDK e2e tests should be run — on `ts-sdk/`, TS e2e test, or Node coordinator changes ("true"/"false") | true | |
600600
| run-ui-tests | Whether UI tests should be run ("true"/"false") | true | |
601601
| run-unit-tests | Whether unit tests should be run ("true"/"false") | true | |
602602
| run-www-tests | Whether Legacy WWW tests should be run ("true"/"false") | true | |

dev/breeze/src/airflow_breeze/utils/selective_checks.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -487,13 +487,19 @@ def __hash__(self):
487487
],
488488
FileGroupForCi.TS_SDK_DOCS_FILES: [
489489
# TypeDoc renders the reference from the SDK sources, and the landing page is
490-
# authored in ts-sdk/docs — unlike TS_SDK_FILES, `.md` counts here.
490+
# authored in ts-sdk/docs — unlike TS_SDK_FILES, `.md` counts here. tsconfig.json
491+
# and package.json are included too: docs/tsconfig.json `extends` the former, and
492+
# the latter pins the `@msgpack/msgpack` version the checked program depends on.
491493
r"^ts-sdk/docs/.*",
492494
r"^ts-sdk/src/.*",
495+
r"^ts-sdk/tsconfig\.json$",
496+
r"^ts-sdk/package\.json$",
493497
],
494498
FileGroupForCi.TS_SDK_FILES: [
495499
# `.md` excluded — doc-only edits do not affect the generated supervisor schema.
496-
r"^ts-sdk/(?!.*\.md$).*",
500+
# `ts-sdk/docs/package.json` and its lock file excluded too — they pin the docs
501+
# toolchain's own dependencies and do not affect the SDK build.
502+
r"^ts-sdk/(?!.*\.md$)(?!docs/package(-lock)?\.json$).*",
497503
],
498504
FileGroupForCi.ASSET_FILES: [
499505
r"^airflow-core/src/airflow/assets/",

dev/breeze/tests/test_selective_checks.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1509,6 +1509,20 @@ def assert_outputs_are_printed(expected_outputs: dict[str, str], stderr: str):
15091509
},
15101510
id="Build ts-sdk docs for a docs-only Markdown change that skips ts-sdk tests",
15111511
),
1512+
pytest.param(
1513+
("ts-sdk/tsconfig.json",),
1514+
{
1515+
"run-ts-sdk-docs": "true",
1516+
},
1517+
id="Build ts-sdk docs when tsconfig.json changes since docs/tsconfig.json extends it",
1518+
),
1519+
pytest.param(
1520+
("ts-sdk/package.json",),
1521+
{
1522+
"run-ts-sdk-docs": "true",
1523+
},
1524+
id="Build ts-sdk docs when package.json changes since it pins @msgpack/msgpack",
1525+
),
15121526
pytest.param(
15131527
("ts-sdk/README.md",),
15141528
{
@@ -1849,6 +1863,16 @@ def test_ktlint_hook_only_runs_for_java_sdk_changes(files: tuple[str, ...], ktli
18491863
True,
18501864
id="skipped when only nested ts-sdk docs change",
18511865
),
1866+
pytest.param(
1867+
("ts-sdk/docs/package.json",),
1868+
True,
1869+
id="skipped when only the docs toolchain's package.json changes",
1870+
),
1871+
pytest.param(
1872+
("ts-sdk/docs/package-lock.json",),
1873+
True,
1874+
id="skipped when only the docs toolchain's lock file changes",
1875+
),
18521876
],
18531877
)
18541878
def test_check_ts_sdk_supervisor_schema_hook_only_runs_for_relevant_changes(

ts-sdk/README.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -189,8 +189,9 @@ current task context when omitted.
189189
## Cancellation
190190

191191
`ctx.signal` is an `AbortSignal` controlled by the active runtime. Pass it to
192-
`fetch()`, timers, database clients, child processes, or other abortable APIs
193-
so tasks can clean up cooperatively when Airflow terminates the task attempt.
192+
`fetch()`, timers, database clients, child processes, or any other API that
193+
accepts an abort signal so tasks can clean up cooperatively when Airflow
194+
terminates the task subprocess with SIGTERM or SIGINT.
194195

195196
## Development
196197

ts-sdk/docs/tsconfig.json

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,20 @@
11
{
2-
"//": "Docs-only tsconfig for the TypeDoc analysis pass. It inherits the SDK's compiler settings verbatim and roots the program at the public entry point only: TypeScript pulls in whatever that entry point transitively imports, but a glob like `../src/**/*.ts` would also root unreachable internal modules (e.g. the `cli/` bin, which needs the optional `esbuild` peer dependency) that the docs toolchain has no reason to install. `paths` below redirects `@msgpack/msgpack`, a real transitive dependency of the checked program (coordinator/frames.ts, reachable from the public startCoordinator export): Node module resolution walks up node_modules from the *importing file* (ts-sdk/src/coordinator/), never sideways into this package's own node_modules, so the bare specifier would otherwise never resolve here.",
2+
"//": [
3+
"Docs-only tsconfig for the TypeDoc analysis pass. It inherits the SDK's compiler",
4+
"settings verbatim and roots the program at the public entry point only: TypeScript",
5+
"pulls in whatever that entry point transitively imports, but a glob like",
6+
"`../src/**/*.ts` would also root unreachable internal modules (e.g. the `cli/` bin,",
7+
"which needs the optional `esbuild` peer dependency) that the docs toolchain has no",
8+
"reason to install. `paths` below redirects `@msgpack/msgpack`, a real transitive",
9+
"dependency of the checked program (coordinator/frames.ts, reachable from the public",
10+
"startCoordinator export): Node module resolution walks up node_modules from the",
11+
"*importing file* (ts-sdk/src/coordinator/), never sideways into this package's own",
12+
"node_modules, so the bare specifier would otherwise never resolve here."
13+
],
314
"extends": "../tsconfig.json",
415
"compilerOptions": {
516
"paths": {
6-
"@msgpack/msgpack": ["./node_modules/@msgpack/msgpack/dist.esm/index.d.ts"]
17+
"@msgpack/msgpack": ["./node_modules/@msgpack/msgpack"]
718
}
819
},
920
"include": ["../src/index.ts"]

0 commit comments

Comments
 (0)