ci: add a PR gate that type-checks the generated code - #59
Merged
Conversation
codegen 机器人开的 PR 此前无任何能作为合并条件的 check：原 workflow.yml 只做覆盖率上报，且挂在裸 [push, pull_request] 上。本次补齐门禁，源码零改动。 为什么这么定： - 新增 Makefile 的 ci-syntax，分两段闸： G1 `tsc --noEmit` —— 这个仓此前完全没有类型检查入口（npm run build 会写 lib/，不适合进门禁），生成代码的正确性无人把关。 G2 dup-check.sh —— tsc 对同一文件内重复的 export interface 走 declaration merging，静默通过。已实测：注入重复 interface 后 tsc 仍 exit 0，dup-check exit 1。故 G1 抓不到重复声明，必须另加显式检测。 - 不设 lint job。tslint 当前 4017 条报错（生成侧 3943 + 手写侧 74）全是风格 问题，无一影响运行——同一批代码 tsc --noEmit exit 0、输出 0 行。加一个恒红 的非阻断 job 只会在每个 PR 上留红叉、制造「CI 坏了」的误解，反而拉低门禁 可信度。生成代码的正确性由 ci-syntax 承担。理由已写进 ci.yml 注释。 - 聚合 job 恒名 ci-gate（needs + if: always()，任一 needs 失败或被取消则 exit 1）。分支保护只需把 ci-gate 配成 required check，其余 job 增删不影响它。 - node 10/12/14 矩阵下移到 compat.yml，只留 schedule + workflow_dispatch。 它回答的是「声明的下界今天还装不装得上」，与某个 PR 改得对不对无关；留在 PR 上会给每次 codegen 提交增加无关 check。fail-fast: false 以便一次看全 哪几个版本还活着。 - 摘掉 codecov 的 fail_ci_if_error: true 并加 continue-on-error。原配置把 第三方服务的可用性接进了合并条件；codecov 挂了不该把 PR 拦死。 - 触发从裸 [push, pull_request] 收窄为 master 的 pull_request + push； 补显式 permissions: contents: read。 源码一行未改：src/services/ 生成代码与 src/core/ 手写代码均未触碰。 特别地未动 src/core/transport/index.ts:50 的 `!=`——getRetCode() 运行时 可能返回字符串，改成 `!==` 会引入真 bug。 本地验证：tsc --noEmit exit 0；dup-check ts src 扫 53 文件 0 重复 exit 0； make test-cov 5 suites / 11 tests 全过 exit 0。反向验证：注入类型错后 tsc exit 1、make ci-syntax exit 2，还原后恢复 exit 0。
上一条 commit 的注释写成「本仓没有提交 package-lock.json」，与事实不符： package-lock.json 和 yarn.lock 都在版本控制里。 实际情况（已核实）：package-lock.json 是 lockfileVersion 1（npm 6 时代格式）， 其 root version 停在 0.2.25，而 package.json 已是 0.2.26，本就不同步。 选 npm install 而非 npm ci 的真实理由是这个——npm ci 要求 lock 与 package.json 严格一致，拿它当门禁第一步会让一个与改动无关的陈旧文件决定 PR 红绿。 只改注释，yml 行为不变。
兼容矩阵是纯巡检，不参与门禁判定，也没人会盯一个非阻断的定时任务。 声明下界还成不成立是另一个问题，要验的时候手工跑一次即可， 不必为它常驻一条每天运行的 workflow。
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
workflow.yml→ci.yml—ci-syntax/test, aggregated byci-gate.github/workflows/compat.yml— the node 10/12/14 matrix, moved off the PR pathscripts/dup-check.shMakefile: newci-syntaxtargetNo source changes.
src/is untouched.The two gates
npx tsc --noEmit— this repo had no type checking in CI at allscripts/dup-check.sh ts src— duplicate exported top-level declarationsG2 is not redundant. Demonstrated on this repo: two
export interface GateProbein one file →There is deliberately no lint job
npm run tslintcurrently reports 4017 errors (3943 in generated code, 74 hand-written). All of them are style. The decisive evidence that none affect behaviour: the same code passestsc --noEmitwith exit 0 and zero output.Adding a permanently-red non-blocking job would just put a red cross on every PR and train people to ignore CI. Correctness of generated code is carried by
ci-syntax. Restoring a lint gate should start by getting those 4017 to zero — not by adding the job first.Other changes
[push, pull_request]tomasteronlyfail_ci_if_error: truedropped from the codecov step, which had made a third-party service's availability part of the merge condition; the upload is nowcontinue-on-errorand is not inci-gate'sneedscompat.ymlruns onschedule+workflow_dispatchonly, so it adds no checks to PRsnpm installis kept overnpm ci: the committedpackage-lock.jsonis lockfileVersion 1 and its root version is 0.2.25 whilepackage.jsonis 0.2.26, so they are already out of sync.npm cirequires them to match exactly, which would let an unrelated stale file decide whether a PR is red.Verified locally
npx tsc --noEmitscripts/dup-check.sh ts srcmake test-covReverse-checked that both gates can actually go red: a type error injected into
src/version.ts→tscexit 1 (TS2322); restored → exit 0.